Express 김수빈 #54
Merged
Express 김수빈 #54
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
basilry
reviewed
Jun 23, 2025
Contributor
basilry
left a comment
basilry
left a comment
There was a problem hiding this comment.
전체적으로 잘 하셨습니다.
백엔드가 상대적으로 프론트보다 분량이 적긴 하죠^^;
고생하셨습니다.
| res.json(articles); | ||
| }; | ||
|
|
||
| // export const createComment = async (req: Request, res: Response) => { |
| throw new HttpError(400, '리프레시 토큰이 필요합니다'); | ||
| } | ||
|
|
||
| // const result = await authService.refreshAccessToken(refreshToken); |
Contributor
There was a problem hiding this comment.
보통 pr이나 코드리뷰 전에는 이런 주석처리된 소스들은 설명을 달거나 삭제합니다.
|
|
||
| const comment = await commentService.createArticleComment( | ||
| userId, | ||
| userId!, |
Contributor
There was a problem hiding this comment.
이게 지속적으로 반복되는데, 이건 컴파일러에게 그냥 속이는게 아닌가요?
만약에 실제로 userId가 해당 서비스 함수에서 원하는 값이 안들어오면 어떻게 하죠?
차라리 아래처럼 타입가드를 처리하는게 낫지 않을까요?
if (typeof userId !== "number") {
throw new HttpError(401, 'Unauthorized');
}
| const products = await productService.getProducts({ | ||
| page: parseInt(page as string, 10), | ||
| pageSize: parseInt(pageSize as string, 10), | ||
| orderBy: orderBy as string | undefined, |
Contributor
There was a problem hiding this comment.
왜 as를 이렇게 많이 쓰셨나요.
강제로 타입캐스팅을 하는 것보다 인터페이스, 타입을 써서 객체에 할당하는게 낫지 않을까요?
아니면 로직을 수정해서 page: typeof page === "string" ? Number(page) : page로 하는게 낫지 않나요?
참고로 parseInt보다 Number가 메모리를 덜 먹고, 더 빠릅니다.
| @@ -10,8 +10,8 @@ router.get('/', errorHandler(articleController.getArticles)); | |||
| router.get('/:articleId', errorHandler(articleController.getArticle)); | |||
Contributor
There was a problem hiding this comment.
보아하니 에러핸들링을 개별 라우터마다 모두 달아두었는데,
이러지 마시고 app.ts에서 맨 마지막 부분쯤에
app.use(globalErrorHandler);
처럼 구성하는게 낫지 않을까요??
| export const updateArticle = async (id: number, data: Pick<Article, 'title' | 'content'>) => { | ||
| const entity = await articleRepository.Update(id, data); | ||
| if (!entity) throw new HttpError(404, '게시글이 존재하지 않습니다'); | ||
| return { |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
요구사항
기본
심화
주요 변경사항
스크린샷
멘토에게
-기존에 구현하지 않은 기능도 있고 코드가 지저분해서 추후 리펙토링 예정입니다 ㅠ
-타입지정만으로도 힘들어서 이번에는 리펙토링을 하지 못햇습니다.