Skip to content

Preparando revisão do código#1

Open
ronaldjunior2 wants to merge 1 commit intocarmoandre:mainfrom
ronaldjunior2:main
Open

Preparando revisão do código#1
ronaldjunior2 wants to merge 1 commit intocarmoandre:mainfrom
ronaldjunior2:main

Conversation

@ronaldjunior2
Copy link

No description provided.

Copy link

@ElderGr ElderGr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fala André, tudo bem? Bora pra mais um code review!
Numa visão geral o projeto ficou com uma boa concisão e semântica. Conseguiríamos desenvolver um pouco mais a arquitetura, dividindo as responsabilidades em camadas, e adicionar algumas boas práticas relacionadas ao typescript. Qualquer dúvida só chamar :D.

pokemonController.alterCatchedPokemonStatus
);

app.get("/populate", pokemonController.populateDatabase);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Outra solução para popular o banco de dados seria criar uma seed, e no ato depois de criar as tabelas teríamos um script para consumir a seed.

Comment on lines -26 to -29
const user = await userRepository.find({
where: { id: userId },
relations: ["userPokemons"],
});
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nesse caso, como buscamos um único registro no banco de dados, poderíamos usar o .findOne(), que retornaria um objeto de User. Assim não precisamos nos preocupar em referenciar índice do array ou usar outras técnicas para consumir o json dentro.


import Session from "../entities/Session";

export async function authenticate(token: string) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seria legal incluir a tipagem ao retorno dessa função, algo como:

export async function authenticate(token: string)Promise<string | null> {...}

Dessa forma, quem consumisse o valor que essa função retorna não precisaria se preocupar com a tipagem da variável que armazenaria esse valor, além de ser uma boa prática quando usamos typescript.

Esse comentário se aplica a situações semelhantes.

Comment on lines -41 to -43
for (let i = 0; i < qtd; i++) {
createdPokemons.push((await createPokemon(pokemonBody())).id);
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Outra alternativa nesse ponto seria usarmos a função Array.from() para substituir o for, algo como:

Array.from({ length: qtd }, (_, index) => Promise.all(createPokemon(pokemonBody( ))).id)

No primeiro parâmetro da função passamos o tamanho do array e na segunda uma função dizemos o que retornaremos em cada iteração do array (semelhante ao .map() ). Ganhamos em semântica e não nos preocupamos em controlar a variavel i.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants