Skip to content

Express 전주형 sprint6#14

Open
JoohyoungJun wants to merge 6 commits intocodeit-sprint-fullstack:express-전주형from
JoohyoungJun:express-전주형-sprint6

Hidden character warning

The head ref may contain hidden characters: "express-\uc804\uc8fc\ud615-sprint6"
Open

Express 전주형 sprint6#14
JoohyoungJun wants to merge 6 commits intocodeit-sprint-fullstack:express-전주형from
JoohyoungJun:express-전주형-sprint6

Conversation

@JoohyoungJun
Copy link
Collaborator

No description provided.

@JoohyoungJun JoohyoungJun requested a review from 11t518s January 23, 2026 10:00
@JoohyoungJun JoohyoungJun self-assigned this Jan 23, 2026
@JoohyoungJun JoohyoungJun marked this pull request as ready for review January 25, 2026 10:42
Copy link
Collaborator

@11t518s 11t518s left a comment

Choose a reason for hiding this comment

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

주형님 고생 많으셨습니다

오늘도.. 본문이 없어서 집중해서 어디를 피드백드리면 좋을까는 정확히 파악하지 못했습니다

한가지 피드백을 드린다면, 개발에는 어느정도 이전 개발자들이 만들어둔 개발 패턴같은게 있습니다!

백엔드 쪽은 mvc라고 하는 고전적인 패턴이 있는데, 한번 그걸 참고해서 코드를 수정해봐도 좋을 것 같습니다!

Comment on lines +37 to +47
CREATE INDEX "Product_createdAt_idx" ON "Product"("createdAt");

-- CreateIndex
CREATE INDEX "Article_createdAt_idx" ON "Article"("createdAt");

-- CreateIndex
CREATE INDEX "Comment_productId_createdAt_idx" ON "Comment"("productId", "createdAt");

-- CreateIndex
CREATE INDEX "Comment_articleId_createdAt_idx" ON "Comment"("articleId", "createdAt");

Copy link
Collaborator

Choose a reason for hiding this comment

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

index 넣어주셨는데, 혹시 어떤경우에 어떻게 index를 넣으면 좋을까요?
복합인덱스는 언제 어떻게 넣으면 좋을까요?
index를 넣는건 무조건 좋은일일까요?
어떤 테이블에는 index가 들어가는게 좋고, 어떤 테이블은 안좋을까요?

요런 고민 한번 해보셨을지 잘 모르겠어서 한번 해봐주셔도 좋을 것 같습니다!

Comment on lines +49 to +52
ALTER TABLE "Comment" ADD CONSTRAINT "Comment_productId_fkey" FOREIGN KEY ("productId") REFERENCES "Product"("id") ON DELETE CASCADE ON UPDATE CASCADE;

-- AddForeignKey
ALTER TABLE "Comment" ADD CONSTRAINT "Comment_articleId_fkey" FOREIGN KEY ("articleId") REFERENCES "Article"("id") ON DELETE CASCADE ON UPDATE CASCADE;
Copy link
Collaborator

Choose a reason for hiding this comment

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

요거는 한번 찾아보시면 좋은게 FK 를 넣는게 좋은가 안좋은가 하는 고민들이 늘 있습니다!

FK 없어도 join 같은게 잘 되긴해서...

FK가 있으면 데이터 지우거나 할 때 에러 떠서 정합성 맞추기에 좋다! 하는 파가 있고
FK 때문에 빠르게 DB에서 칼럼 지우거나 하는거 처리 속도가 늦어진다 하는 쪽이 있습니다!

요거는 한번 찾아봐보시면 좋을 것 같아서 코멘트 남깁니당~

Comment on lines +16 to +23
if (!content) {
return res.status(400).json({ message: 'content가 없습니다.' });
}
if (!article) {
return res
.status(404)
.json({ message: '요청한 게시물이 존재하지 않습니다.' });
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

이런 얼리리턴 좋습니다!

혹시 이걸 더 편하게 사용할 수 있는 패턴이나 방법론이 있을까요?

Comment on lines +60 to +74
const findArgs = {
where: { articleId },
orderBy: [{ createdAt: 'desc' }, { id: 'desc' }],
take: limit + 1,
select: {
id: true,
content: true,
createdAt: true,
},
};

if (cursor) {
findArgs.cursor = { id: cursor };
findArgs.skip = 1;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

이런 코드들이 들어오니까 요 함수가 좀 길어지고 맥락이 많아지는 것 같습니다. 혹시 요런걸 별도로 만들어서 관리한다면 어떤 패턴이 좋을까요?

Comment on lines +133 to +136
const offset = Number(req.query.offset ?? 0);
const limit = Number(req.query.limit ?? 10);
const keyword = String(req.query.keyword ?? '');
const orderBy = String(req.query.orderBy ?? 'recent');
Copy link
Collaborator

Choose a reason for hiding this comment

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

이렇게 String, Number를 해주는 방식이 좋을까요? 아니면 error 를 떤지는게 좋을까요?

@11t518s
Copy link
Collaborator

11t518s commented Jan 26, 2026

@JoohyoungJun 이거 conflict 잡아주시면 머지 눌러둘게요!

@JoohyoungJun JoohyoungJun force-pushed the express-전주형-sprint6 branch from 6a5543d to ecd5960 Compare January 27, 2026 01:36
@JoohyoungJun
Copy link
Collaborator Author

conflict 해결했습니다!

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