revise meu código segundo o clean code 🦾
Estou fazendo um curso na RocketSeat e recebi esse código para fazer o clean code:
// Nomenclatura de variáveis
const list = [
{
title: 'User',
followers: 5
},
{
title: 'Friendly',
followers: 50,
},
{
title: 'Famous',
followers: 500,
},
{
title: 'Super Star',
followers: 1000,
},
]
export default async function getData(req, res) {
const github = String(req.query.username)
if (!github) {
return res.status(400).json({
message: `Please provide an username to search on the github API`
})
}
const response = await fetch(`https://api.github.com/users/${github}`);
if (response.status === 404) {
return res.status(400).json({
message: `User with username "${github}" not found`
})
}
const data = await response.json()
const orderList = list.sort((a, b) => b.followers - a.followers);
const category = orderList.find(i => data.followers > i.followers)
const result = {
github,
category: category.title
}
return result
}
getData({ query: {
username: 'josepholiveira'
}}, {})
e esse foi o que fiz, gostaria de um feedback de vocês se estou no caminho certo, sou júnior e atualmente estou criando e mantendo um projeto sozinha, tenho procurado ter mais conhecimento para aprimorar meus conhecimentos.
// Nomenclatura de variáveis
// API que recebe uma lista de contas no github e retorna o que ela é segundo o número de estrelas que ela tem
const listRolesOfUserNamesGithub = [
{
title: "User",
followers: 5,
},
{
title: "Friendly",
followers: 50,
},
{
title: "Famous",
followers: 500,
},
{
title: "Super Star",
followers: 1000,
},
];
interface GithubRequest {
query: {
username: string;
};
status?: number;
}
interface GithubResponse {
status: (code: number) => GithubResponse;
json: (body: any) => void;
}
export default async function getDataUsersGithub(
requestAPIGithub: GithubRequest,
responseAPIGithub: GithubResponse
) {
console.log(requestAPIGithub);
const githubUsername = String(requestAPIGithub.query.username);
if (!githubUsername) {
return responseAPIGithub.status(400).json({
message: `Please provide an username to search on the github API`,
});
}
const githubUserResponse = await fetch(
`https://api.github.com/users/${githubUsername}`
);
if (githubUserResponse.status === 404) {
return responseAPIGithub.status(400).json({
message: `User with username "${githubUsername}" not found`,
});
}
const dataAPIUsers = await githubUserResponse.json();
const orderListOfUsers = listRolesOfUserNamesGithub.sort(
(firstRole, secondRole) => firstRole.followers - secondRole.followers
);
const categoryOfUsers = orderListOfUsers.find(
(index) => dataAPIUsers.followers > index.followers
);
const resultListForOrderOfFollowers = {
githubUsername,
category: categoryOfUsers.title,
};
return resultListForOrderOfFollowers;
}
console.log(
getDataUsersGithub(
{
query: {
username: "josepholiveira",
},
},
{
status: (code: number) => {
console.log(`Status: ${code}`);
return this;
},
json: (body: any) => {
console.log(`Response JSON:`, body);
},
}
)
);
Agradeço de antemão a todos que me ajudarem!
1 Coríntios 10:31
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:
// Crie os itens na ordem correta, assim não precisa mais ordenar
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:
// procura o critério
const criteria = categoriesCriteria.find(i => data.followers > i.followers);
let category;
if (criteria) { // se encontrou, usa o valor encontrado
category = criteria.title;
} else { // se não encontrou, não tem categoria
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:
// mantive as interfaces, mas conforme já dito acima, pode nem ser necessária dependendo do caso
interface GithubRequest {
query: {
username: string;
};
status?: number;
}
// response contém o status e os dados
// os dados podem ser a mensagem de erro ou o objeto contendo a categoria e username
// como melhoria, poderia criar interfaces para cada um destes casos
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 } // se for menor que 5, não tem categoria
];
// a função recebe um request e retorna um response
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);
// retorna o objeto diretamente, sem precisar criar uma variável só para retorná-la em seguida
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'}});
// usar/verificar o response, etc
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...
Fiquei curioso para saber qual seria o código ideal segundo os ditames do Clean Code. Caso seu (possível) instrutor, josepholiveira, permita, e se puder, compartilhe conosco qual foi, elencando os pontos principais desse conjunto de boas práticas para escrita de um código.
Clean code nesse tipo de caso (código de exemplo) é uma coisa complicada... Você vai refazer todo o código bonitinho seguindo os princípios do clean code e depois vai pensar: "Que exagero é esse?!".
Mas vamos lá. Um dos princípios fundamentais do clean code é o SRP e eu senti falta disso. Esse código faz a validação do input e também o fetch para a API do GitHub. Minha sugestão: quebrar em serviços.
Imagine que você vai precisar validar esse nome de usuário em outros lugares ou que as validações vão ficar mais complexas. Joga essa lógica em um serviço e depois injeta ele nesse consumidor.
O mesmo para a chamada da API. No código é algo bem simples (novamente porque é só um código de exemplo), mas na vida real você teria que pensar em autenticação, filtragem, paginação, mapeamento, tratamento de erros, estratégias de resiliência e etc.