-
Notifications
You must be signed in to change notification settings - Fork 5
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
체크리스트 질문 조회 API를 구현한다. #83
Conversation
Co-Authored-By: tkdgur0906 <[email protected]>
Co-Authored-By: tkdgur0906 <[email protected]>
Co-Authored-By: tkdgur0906 <[email protected]>
Co-Authored-By: tkdgur0906 <[email protected]>
Co-Authored-By: tkdgur0906 <[email protected]>
Co-Authored-By: tkdgur0906 <[email protected]>
Co-Authored-By: tkdgur0906 <[email protected]>
Co-Authored-By: tkdgur0906 <[email protected]>
Co-Authored-By: tkdgur0906 <[email protected]>
public String getTitleByQuestionId(int questionId) { | ||
return questions.get(questionId).getTitle(); | ||
} | ||
|
||
public String getSubtitleByQuestionId(int questionId) { | ||
return questions.get(questionId).getSubtitle(); | ||
} |
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.
getQuestion으로 가져와서 q.getSubtitle() / q.getTitle()을 쓰는 건 어떻게 생각하시나요?
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.
현재 구조에서는 이 방식이 더 합리적인 것 같아 채택하였는데 추후 재활용성이 떨어지면 리팩토링하는 방식으로 가 보겠습니다!
private void readChecklistQuestion(Category category, List<QuestionResponse> questionResponses) { | ||
category.getQuestionIds().stream() | ||
.map(questionId -> new QuestionResponse(questionId, | ||
questionList.getTitleByQuestionId(questionId), | ||
questionList.getSubtitleByQuestionId(questionId))) | ||
.forEach(questionResponses::add); | ||
} |
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.
return type을 List로 주는 건 어떨까요?
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.
간단히 코멘트 남겼습니다!
readChecklistQuestion메서드는 시간남을 때 조금 더 리팩토링해보면 좋을 것 같아요 😃
|
||
private final int id; | ||
private final String description; | ||
private final List<Integer> questionIds; |
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.
중복제거할 수 있는 Set 자료구조가 더 적합해보여요
Co-Authored-By: tkdgur0906 <[email protected]>
모두 반영 완료하였습니다~ |
❗ Issue
✨ 구현한 기능
📢 논의하고 싶은 내용
기존 설계에서는 TINYINT를 적용하기로 하였으나 클래스 설정을 INTEGER로 하여 해당 문제가 발생함.
논의 후 변경 예정이다.
🎸 기타