[BE] feat: 하이라이트 정보 함께 조회 #876
Conversation
Test Results155 tests 152 ✅ 5s ⏱️ Results for commit ab0e6b2. ♻️ This comment has been updated with latest results. |
| @@ -35,7 +38,14 @@ public ReviewsGatheredBySectionResponse getReceivedReviewsBySectionId(ReviewGrou | |||
| Section section = getSectionOrThrow(sectionId, reviewGroup); | |||
| Map<Question, List<Answer>> questionAnswers = getQuestionAnswers(section, reviewGroup); | |||
|
|
|||
| return reviewGatherMapper.mapToReviewsGatheredBySection(questionAnswers); | |||
| List<Long> answerIds = questionAnswers.values() | |||
There was a problem hiding this comment.
중복없는 answerId의 집합이어야 하니 Set은 어떨까요?
There was a problem hiding this comment.
distinct 추가하겠습니다 🫡
There was a problem hiding this comment.
set으로 제한하지 않고 list로 하는 건 set의 활용이 제한적이라 그런건가요?
어차피 distinct하면 결과는 똑같긴 하지만요!
| private List<HighlightResponse> mapToHighlightResponse(List<Highlight> highlights) { | ||
| // Line index를 기준으로 묶되, 묶은 것들은 mapping 함수를 통해 List로 변환 | ||
| Collector<Highlight, ?, Map<Integer, List<RangeResponse>>> highlightMapCollector = Collectors.groupingBy( | ||
| Highlight::getLineIndex, | ||
| Collectors.mapping(highlight -> RangeResponse.from(highlight.getHighlightRange()), Collectors.toList()) | ||
| ); | ||
| Map<Integer, List<RangeResponse>> lineIndexRangeResponses = highlights.stream() | ||
| .collect(highlightMapCollector); | ||
|
|
||
| return lineIndexRangeResponses.entrySet() | ||
| .stream() | ||
| .map(entry -> new HighlightResponse(entry.getKey(), entry.getValue())) | ||
| .toList(); | ||
| } |
There was a problem hiding this comment.
Collector<Highlight, ?, Map<Integer, List<RangeResponse>>> highlightMapCollector
이 부분이 이해하는 데 좀 시간이 걸렸던 것 같아요. Collector를 받아서 한번더 collect하는 부분을 합쳐서 아래처럼 해도 같은 동작이지 않을까 싶어요!
Map<Integer, List<RangeResponse>> lineIndexRangeResponses = highlights.stream()
.collect(Collectors.groupingBy(
Highlight::getLineIndex,
Collectors.mapping(highlight -> RangeResponse.from(highlight.getHighlightRange()), Collectors.toList())
));
return lineIndexRangeResponses.entrySet()
.stream()
.map(entry -> new HighlightResponse(entry.getKey(), entry.getValue()))
.toList();There was a problem hiding this comment.
처음에 합쳐서 썼다가 좀 어려워서 풀어뒀는데, 오히려 변수 타입 때문에 어려워졌겠네요..,
분리할 때 최초에는 var를 쓸까 잠시 고민했었습니다 ㅋㅋ
There was a problem hiding this comment.
기존 코드에 덧대었습니다. 구조를 만지고 싶지만.. 이건 지금 할 일은 아닌 듯하네요.
최대한 코드 수정을 안 했어요. 변경 사항이 눈에 확 들어왔으면 좋겠습니다.
우리의 상황에 적절한 코드 변경이었다 생각해요,
좋은 판단이었습니다👏
이해하기 쉬운가요?
솔직히 이해가 쉽진 않았습니다..😅
원인을 생각해보니,
섹션 - 섹션 하위의 질문들
질문 - 질문 하위의 답변들
답변 - 답변 하위의 라인들
라인 - 라인 하위의 하이라이드 범위들
이런식으로 트리 구조이기 때문에
Map<Object, Object> 또는 List 형식을 만들고,
이를 통해서 response dto 를 반환하는 private 함수가 생기는게 원인인 것 같아요.
그리고 그 과정에서 생기는 연속적인 steam 도 코드를 읽는데 피로하게 만드는 것 같고요😵💫
이제 진지하게 조회 구조를 바꿔야 할 때가 온 것 같습니다😞
정규화 테이블과 JSON 저장 테이블을 동시에 두거나 하는 방법으로요..
무튼 아루 고맙습니당
호프스카치로도 테스트 해봤는데 잘 되네요!
| public record RangeResponse( | ||
| long startIndex, | ||
| long endIndex | ||
| ) { | ||
| public static RangeResponse from(HighlightRange range) { | ||
| return new RangeResponse(range.getStartIndex(), range.getEndIndex()); | ||
| } |
There was a problem hiding this comment.
리뷰미 첫 dto 의 static 메서드군요..! 👏
dto 의 깊이가 길어짐에 따라서
- mapper 의 private 메서드의 깊이가 깊어지는 것
- or mapper 함수 길이가 길어지는 것을 막을 수 있을 것 같아요.
저는 이런 구조 찬성입니다🙆🏻♀️
그런데 이것도 클래스 시작 개행이 적용되어야 할 것 같아요~
이것도 대장정이 되겠네요 ㅋㅋ |
* feat: 하이라이트 정보 함께 조회 * fix: 테스트 컴파일 에러 해결 * refactor: 하이라이트 순서대로 정렬 * chore: remove unused import * test: 오름차순 테스트 * refactor: `answerIds` 중복 제거 * refactor: groupingby 다시 합쳐두기 * style: 클래스 개행
🚀 어떤 기능을 구현했나요 ?
🔥 어떻게 해결했나요 ?
📝 어떤 부분에 집중해서 리뷰해야 할까요?
Collector.groupingBy메서드가 참 야무집니다. 묶어서 매핑해주는 친군데 이번에 활용해 보았어요. 좀 길어진다 싶은 한 군데에 주석으로 표기해 두었습니다.