Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

preparando o review #9

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

preparando o review #9

wants to merge 1 commit into from

Conversation

driven-tutores
Copy link

Fala ai, beleza?
Só avisando que esse é o PR do feedback de código desse projeto.
Por favor, não fecha nem recuse esse pull request até a gente liberar o restante dos comentários, combinado?
Fica no aguardo que em breve mostro os principais pontos de melhoria no seu código.

Copy link
Author

@driven-tutores driven-tutores left a comment

Choose a reason for hiding this comment

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

Fala ai Andrezza, Galdino passando aqui pra mandar mais um review de código.
Gostei bastante da execução do seu projeto, você aplicou bem os conceitos de interface e fez e usou bem a tipagem do typescript, parabéns!
Acho que o principal ponto de melhoria no seu código é o mesmo do sing me a song, onde tem bastante código repetido na parte das validações, essa repetição poderia ser simplificada lançando erros!

Comment on lines -9 to -24
if (
!question.question ||
!question.student ||
!question.class ||
!question.tags
) {
return res.sendStatus(400);
}

const questionFormat = await questionsService.validateQuestion(question);

if (questionFormat) {
return res
.status(400)
.send({ message: questionFormat.details[0].message });
}
Copy link
Author

Choose a reason for hiding this comment

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

Você podia ter levado esse if que checa se os campos exigidos na requisição estão corretos pra dentro de questionService.validateQuestion. Nessa função você poderia faze rum throw que lança um erro customizado chamado de BodyError ou RequestError, isso evitaria fazer esses ifs nas controllers e limparia sua lógica.
A ideia é ter a menor quantidade possível de código em controllers, de preferência sem nenhum if, apenas um try/catch e a chamada das funções necessárias dentro do try.

Comment on lines -87 to -89
const getSingleQuestionById = await questionsService.getSingleQuestionById(
numId
);
Copy link
Author

Choose a reason for hiding this comment

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

Esse repository não está retornando todos os detalhes das perguntas, faltando dados das answers.

Comment on lines -74 to -76
const FormattedDate = dayjs(getSingleQuestionByIdFromDB?.submitAt).format(
'YYYY-MM-DD HH:mm'
);
Copy link
Author

Choose a reason for hiding this comment

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

Esse trecho foi repetido várias vezes nesse arquivo. Acho que você poderia ter feito uma função pra abstrair essa parte de gerar a formatação da data.

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.

None yet

1 participant