feat: preparando o review #17
Conversation
|
|
||
| return res.status(httpStatusCode.OK).send(question); | ||
| } catch (error) { | ||
| if (error instanceof Invalid) return res.status(httpStatusCode.BAD_REQUEST).send(error.message); |
There was a problem hiding this comment.
Ao invés de fazer essa série de ifs seria mais fácil definir o código de erro dentro do construtor da classe de erro, dessa forma você poderia facilmente ler e retornar esse código de erro como resposta da requisição.
O interessante de fazer dessa forma é que evitamos criar um novo if sempre que um novo tipo de erro customizado for adicionado, lembra que o código tem que permitir que seja fácil expandir e melhorar o projeto.
// na classe
class Invalid extends Error {
constructor(message?: string) {
super(message);
this.name = 'UnauthorizedError';
this.statusCode = httpStatusCode.BAD_REQUEST;
}
}
// no bloco catch
if('statusCode' in t){ // checando se o erro tem statusCode, pra saber se é um erro customizado
return res.status(err.statusCode).send(err.message);
}
return next();
Outra dica, você poderia levar esse ifo que checa se o erro é um erro customizado lá pra sua middleware, dessa forma você não precisa repetir esse if em todos os blocos try/catch da sua aplicação.
| export async function postQuestionAnswer(req: RequestAuthentication, res: Response, next: NextFunction) { | ||
| try { | ||
| const questionId = Number(req.params.id); | ||
|
|
||
| if (!Number.isInteger(questionId) || questionId < 1) { | ||
| throw new Invalid('Invalid Question Id'); | ||
| } | ||
|
|
||
| const { error: invalidBody } = createAnswerSchema.validate(req.body); | ||
| if (invalidBody) { | ||
| throw new Invalid(invalidBody.message); | ||
| } | ||
|
|
||
| const answer: Answer = { | ||
| userId: req.userId, | ||
| questionId, | ||
| answer: req.body.answer, | ||
| }; | ||
|
|
||
| const answerId = await questionsService.createAnswer(answer); | ||
|
|
||
| return res.status(httpStatusCode.CREATED).send({ | ||
| message: `Successfully Answered - Answer Id: ${answerId}.`, | ||
| }); | ||
| } catch (error) { | ||
| if (error instanceof Invalid) return res.status(httpStatusCode.BAD_REQUEST).send(error.message); | ||
| if (error instanceof NotFound) return res.status(httpStatusCode.NOT_FOUND).send(error.message); | ||
| return next(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Essas controllers estão ficando um pouco grandes. Acho que você poderia criar um service se validação pra extrair parte dessa lógica, tirando aqueles dois ifs do começo e deixando a controller mais simples.
A recomendação é que a controller seja o mais simples e o menor possível, apenas chamando as funções necessárias e retornando a resposta para o usuário.
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.