-
Notifications
You must be signed in to change notification settings - Fork 4
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
[BE] 체크리스트 리스트 조회 API를 리팩토링한다. #618
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수고하셨습니다!
(+) 코드를 살펴보다가 본문의 질문을 놓쳤네요 😖
- 저는 save를 제외한 메서드들은 다른 요소들의 조합이 key가 될 가능성이 있기 때문에 ById를 명시해 주는 것이 좋다 생각합니다!
- 저도 카피와 같이 isLikedChecklist 추천해 봅니다
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- updateChecklist 한표요~
- isLikedChecklist 어떤가요~
# Conflicts: # backend/bang-ggood/src/main/java/com/bang_ggood/checklist/controller/ChecklistController.java # backend/bang-ggood/src/main/java/com/bang_ggood/checklist/dto/response/UserChecklistsPreviewResponse.java # backend/bang-ggood/src/main/java/com/bang_ggood/checklist/service/ChecklistManageService.java # backend/bang-ggood/src/main/java/com/bang_ggood/checklist/service/ChecklistService.java # backend/bang-ggood/src/main/java/com/bang_ggood/like/service/ChecklistLikeService.java # backend/bang-ggood/src/test/java/com/bang_ggood/auth/service/AuthServiceTest.java # backend/bang-ggood/src/test/java/com/bang_ggood/checklist/service/ChecklistManageServiceTest.java # backend/bang-ggood/src/test/java/com/bang_ggood/checklist/service/ChecklistServiceTest.java
private List<ChecklistPreviewResponse> mapToChecklistsPreview(List<Checklist> checklists) { | ||
return checklists.stream() | ||
.map(this::mapToChecklistPreview) | ||
.toList(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이정도 코드도 메서드 분리가 필요할까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
분리했을 때 가독성이 훨씬 좋아진다고 생각했습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-> 메서드 삭제했습니다.
ChecklistPreviewResponse previewResponse1 = response.checklists().get(0); | ||
ChecklistPreviewResponse previewResponse2 = response.checklists().get(1); | ||
|
||
assertThat(previewResponse1.checklistId()).isEqualTo(checklist2); // 최신순으로 조회 | ||
assertThat(previewResponse2.checklistId()).isEqualTo(checklist1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
containsExactly를 활용해보는 건 어떨까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
containsExactly를 사용할 때 객체의 동등성으로 확인하게 되는데 현재 구현방식으로는 동등성을 확인하기 어려워 id값을 확인하는 방식으로 구현했습니다.
Long checklist1 = checklistManageService.createChecklist(user, checklistRequest1);
Long checklist2 = checklistManageService.createChecklist(user, checklistRequest2);
반환값이 id값여서 객체의 동등성을 확인하기 어려워요 🥲
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-> 변수명을 checklist -> checklistId
로 변경했습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updateChecklist 한표
isliked 한표
굳
❗ Issue
✨ 구현한 기능
📢 논의하고 싶은 내용
🎸 기타