[BE] 체크리스트 카테고리별 질문 비교 기능을 구현한다. #963
Open
+238
−29
Conversation
This file contains 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
tsulocalize
requested changes
Nov 16, 2024
Comment on lines
+150
to
+151
| Assertions.assertThat(good.get(0).getQuestionId()).isEqualTo(checklist1Question2Good.getQuestionId()); | ||
| Assertions.assertThat(bad).hasSize(1); |
There was a problem hiding this comment.
static import를 쓰면 가독성이 올라갈 것 같네요!
| private List<QuestionResponse> categorizeQuestionsByAnswer(ChecklistQuestions checklistQuestions, Answer answer) { | ||
| return checklistQuestions.filterByAnswer(answer) | ||
| .stream() | ||
| .map(checklistQuestion -> new QuestionResponse(checklistQuestion.getQuestion(), questionService.readHighlights(checklistQuestion.getQuestionId()))) |
There was a problem hiding this comment.
Highlight가 비교 페이지에서 꼭 필요하지 않다면 가져오지 않는 방향으로 가는 게 더 좋아보이긴 합니다!
tkdgur0906
requested changes
Nov 16, 2024
Comment on lines
+65
to
+67
| public boolean hasAnswer(Answer answer) { | ||
| return this.answer.equals(answer); | ||
| } |
There was a problem hiding this comment.
hasAnswer라는 메서드 명이 그냥 Answer를 가지고 있는지 여부를 반환하는 의미로 해석될 수도 있을 것 같아요!
matchesAnswer나 다른 함수명을 고려해보는 것은 어떤가요?
There was a problem hiding this comment.
matchAnswer로 변경했습니다!
|
|
||
| import java.util.List; | ||
|
|
||
| public record ComparisonCategoryChecklistQuestionResponse(List<QuestionResponse> good, |
There was a problem hiding this comment.
dto이름이 한눈에 와닿지 않는 것 같은데, 가지고 있는 데이터를 이름에 담는 방향은 어떻게 생각하시나요?
AnswerCategorizedQuestionsResponse 이런 느낌으로 생각해봤는데 정확하게 어떤게 좋을지는 잘 모르겠네요🤔
Comment on lines
+149
to
+153
| Assertions.assertThat(good).hasSize(1); | ||
| Assertions.assertThat(good.get(0).getQuestionId()).isEqualTo(checklist1Question2Good.getQuestionId()); | ||
| Assertions.assertThat(bad).hasSize(1); | ||
| Assertions.assertThat(bad.get(0).getQuestionId()).isEqualTo(checklist1Question1Bad.getQuestionId()); | ||
| Assertions.assertThat(none).isEmpty(); |
shin-jisong
reviewed
Nov 18, 2024
| @@ -34,6 +36,13 @@ public ResponseEntity<CategoryCustomChecklistQuestionsResponse> readAllCustomChe | |||
| return ResponseEntity.ok(questionManageService.readAllCustomChecklistQuestions(user)); | |||
| } | |||
|
|
|||
| @GetMapping("/v1/comparison/checklists/{checklistId}/categories/{categoryId}/questions") | |||
There was a problem hiding this comment.
queryString으로 설계하지 않고 계층적으로 설계하신 이유가 궁금해요!
계층적 설계가 들어가서 checklists가 먼저 나오니 QuestionController에 위치한 게 어색하다는 생각도 들어서요~
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
❗ Issue
✨ 구현한 기능
해당 커밋으로 확인해주세요
📢 논의하고 싶은 내용
🎸 기타