Primeiramente, estou assumindo que "clean code" se refere ao famoso livro do Uncle Bob. Acho que vale lembrar que este livro é bem opinativo, além de várias coisas serem meio vagas e genéricas, e muito dependentes de contexto. Ou seja, não é pra seguir à risca, como se fosse uma lei universal imutável.
Aliás, isso vale para qualquer "boa prática" que se vê por aí. Qualquer regrinha dessas (principalmente se disser "sempre use isso" ou "nunca faça tal coisa") não deve ser levada ao extremo. Sempre tem que analisar o contexto, os prós e contras, pensar a respeito e só então decidir se vai usar ou não.
Dito isso, não sei se precisa colocar um nome tão longo quanto listRolesOfUserNamesGithub
. Pode não parecer, mas dar bons nomes é uma das coisas mais difíceis na nossa área, talvez por ser algo muito opinativo (cada um tem um critério pessoal). Enfim, acho que esse nome não está bom, pois isso não parece ter relação nenhuma com "roles of user names". Afinal, "role" quer dizer "função", ou seja, geralmente é algo associado com cada usuário (ex: em um sistema, um administrador do sistema pode ser um usuário que tem role de "admin", outro usuário pode ter role de "auditor", podendo auditar contas de outros usuários, etc).
Mas este array não parece ter relação com funções de cada usuário, e sim com os critérios de classificação destes, baseado na quantidade de seguidores. Talvez rankings
ou categories
(ou ainda categoriesCriteria
("critérios das categorias")) sejam nomes melhores.
Outra coisa, toda vez que a função é chamada, vc ordena o array. Mas como o array é global (está fora da função), talvez fosse interessante ordenar apenas uma vez, ou então já crie ele com a ordenação desejada:
const categoriesCriteria = [
{ title: 'Super Star', followers: 1000 },
{ title: 'Famous', followers: 500 },
{ title: 'Friendly', followers: 50 },
{ title: 'User', followers: 5 }
];
Assim, vc não precisa mais usar sort
dentro da função, pois o array sempre estará na ordem correta.
Faria sentido usar sort
se:
- o array fosse modificado em algum ponto
- o array fosse passado como parâmetro da função (pois aí vc não teria a garantia de que está na ordem correta)
Mas como neste caso o array é criado fora da função e ninguém o modifica, acho mais simples criar na ordem certa para que não seja preciso ordená-lo toda hora.
E isso não tem nada a ver com clean code, é apenas lógica pura e simples :-)
Outra coisa que eu recomendo (e que também não tem a ver com clean code em si) é usar ponto-e-vírgula no final das linhas. Pode parecer "frescura", e sei que o JavaScript "aceita" o código sem ponto-e-vírgula e "funciona", mas isso evita algumas situações bizarras que podem ocorrer se você não usá-los, como essa e essa (veja mais sobre isso aqui).
Sobre os demais nomes, também acho discutível usar nomes como requestAPIGithub
e responseAPIGithub
. Pois se a função chama getDataUsersGithub
, pra mim já fica claro que tanto o request quanto o response são do GitHub. E não vejo porque ser tão verboso, pois afinal nomes como req
e res
já são tão comuns no contexo Web que "todo mundo" entende que eles correspondem ao request e response de uma requisição HTTP.
O mesmo vale para githubUsername
. Se estou dentro da função que busca os dados no GitHub, apenas username
já seria o suficiente. Afinal, tudo dentro da função está relacionado ao GitHub, adicionar github
em todas as variáveis acaba ficando redundante, sendo portanto uma verbosidade desnecessária. Não estou dizendo que verbosidade em si é algo ruim, apenas que nem sempre ela é necessária. Tem vezes em que "menos é mais".
E novamente, o nome da função não me parece adequado. Vc não está buscando por dados dos usuários ("get data users"), e sim pela categoria de um usuário específico. Então talvez o nome devesse ser getGitHubCategory
ou algo do tipo.
Também achei exagero criar a interface GithubRequest
, afinal, a função só precisa do username. Faria sentido se esta interface fosse ser usada em outras funções, para diferentes tipos de buscas, com mais campos além do username. Mas se tiver apenas esta função e nada mais, é um típico exemplo de "Você não vai precisar disso".
Outra coisa é que a função pode retornar um GithubResponse
ou um objeto resultListForOrderOfFollowers
que contém o username e a categoria. Talvez esses dados devessem estar dentro do response, assim o tipo de retorno seria apenas um. Mais ainda, pra que passar o response como parâmetro? A função poderia simplesmente criar um e pronto. Aproveitando, resultListForOrderOfFollowers
também não me parece um bom nome, pelos motivos já citados. Poderia mudar para result
apenas (afinal, sabemos que é o resultado da função, cujo nome sugerido - getGitHubCategory
- já nos diz o que ela retorna), ou se quiser muito deixar mais claro, userCategory
ou algo do tipo.
Por fim, se a quantidade de followers
for menor ou igual a 5, nenhuma categoria será encontrada (find
retornará undefined
). Então tem que tratar este caso também. Poderia ser algo assim:
const criteria = categoriesCriteria.find(i => data.followers > i.followers);
let category;
if (criteria) {
category = criteria.title;
} else {
category = 'None';
}
return { username, category };
Ou então adicione uma nova entrada no array de critérios:
const categoriesCriteria = [
{ title: 'Super Star', followers: 1000 },
{ title: 'Famous', followers: 500 },
{ title: 'Friendly', followers: 50 },
{ title: 'User', followers: 5 },
{ title: 'None', followers: -1 }
];
Assim, mesmo se followers
for zero, sempre encontrará algum critério (assumindo que este valor nunca é negativo).
Enfim, juntando tudo que foi dito acima, ficaria assim:
interface GithubRequest {
query: {
username: string;
};
status?: number;
}
interface GithubResponse {
status: number;
data: any;
}
const categoriesCriteria = [
{ title: 'Super Star', followers: 1000 },
{ title: 'Famous', followers: 500 },
{ title: 'Friendly', followers: 50 },
{ title: 'User', followers: 5 },
{ title: 'None', followers: -1 }
];
async function getGitHubCategory(req: GithubRequest): GithubResponse {
const username = req.query.username;
if (!username) {
return {
status: 400,
data: { message: "Please provide an username to search on the github API" }
};
}
const response = await fetch(`https://api.github.com/users/${username}`);
if (response.status === 404) {
return {
status: 400,
data: { message: `User with username "${username}" not found` }
};
}
const data = await response.json();
const criteria = categoriesCriteria.find(i => data.followers > i.followers);
return {
status: 200,
data: { username, category: criteria.title }
};
}
E para usar a função, vc passa o request como parâmetro e atribui o response em uma variável:
var response = await getGitHubCategory({query: {username: 'fulano'}});
Enfim, não sei se isso segue à risca a cartilha do clean code (mas eu já disse que ele não é uma lei universal, é no máximo um conjunto de recomendações que devem ser avaliadas conforme o contexto). Pior ainda, não sei se é isso que o instrutor do curso espera (vai que ele quer muito que os nomes sejam longos e ultra-descritivos, por exemplo; sei lá). Mas a princípio é assim que eu começaria em um sistema real - que, claro, depois tem que ser revisto conforme os requisitos.
Afinal, sempre tem o que melhorar. Como já disseram, talvez devesse separar o fetch
e a verificação em duas funções, por exemplo, para deixar tudo mais modularizado. Mas isso apenas se já estiver previsto que o sistema vai crescer e cada uma dessas partes fosse ser reutilizada (se for apenas essa função e nada mais, aí talvez não valha o esforço), etc etc etc...