#317 [feat] 엔티티 변경으로 인한 종합 일정 시간표 로직 리팩토링 #324
Conversation
기존 코드 때문에 Builder를 없애지 못했습니다. 전체 삭제 할 때 삭제하면 좋을 것 같습니다
|
커밋을 쪼개다가 일부가 반영이 안되었네요 ㅎㅎ 😅 |
| public record TimeSlotRetrieveDto( | ||
| String time, | ||
| List<String> userNames, | ||
| int colorLevel | ||
| ) { |
There was a problem hiding this comment.
P5
지극히 개인적인 의견입니다.
현재 저희가 TimeSlot은 enum으로 정하고 있어서, TimeSlot이라 하니까 시간만 정의 되어 있는 느낌이 든다고 생각합니다..
그래서 TimeSlot 말고 TimeBlock이란 네이밍은 어떻게 생각하시나요??
|
|
||
| public int setColorLevel(final int memberCount, final int availableUserCount) { | ||
| double ratio = (double) availableUserCount / memberCount; |
There was a problem hiding this comment.
p2
접근자가 public입니다! 해당 클래스에서만 사용하는 것 같으니 private으로 바꿔주세요!
| Collectors.mapping(t -> new TimeSlotRetrieveDto( | ||
| t.timeSlot().getTime(), | ||
| userRetrieveService.getUserNamesFromId(t.userIds()), | ||
| setColorLevel(totalUserCount, t.userIds().size()) | ||
| ), | ||
| Collectors.toList() | ||
| ) |
There was a problem hiding this comment.
P3
timeBlock 별로 userRetrieveService.getUserNamesFromId(t.userIds()),을 한다면,
timeBlock 별로 쿼리가 발생해서
최대 266개의 쿼리가 발생하지 않을까 우려가 됩니다..!
그래서 저는 일전에 최적의 회의 시간 로직 리팩토링할 때, 유저를 전체 조회한 후 Map<Long, User>을 통해서 user정보를 가지고 왔었습니다..!
혹시 쿼리가 얼마나 생기는지 알 수 있을까요??
userNames 를 구할 때, 영속성의 도움을 받아서 해당 쿼리가 발생하지 않는 것인가? 란 추측도 하는 중인데.. 실행해보지 않아서 모르겠네요..!
There was a problem hiding this comment.
꼼꼼히 봐주셔서 감사합니다!! 쿼리문이 최대 252개가 나갈 수 있는 상황이었더라고요! 저도 원용님 방식으로 수정하였습니다!
그런데, Map을 사용할경우 정렬이 보장이 되지 않아 테스트를 통과하지 않아서 이를 받아와 sorted하는 방식을 사용했습니다! 리뷰 부탁드립니다!
| ) | ||
| ); | ||
| TimeTableRetrieveDto expected = TimeTableRetrieveDto.of( | ||
| List.of(new String[]{"KWY", "DSH"}), |
There was a problem hiding this comment.
p4
List.of("KWY", "DSH")을 해도 충분할 것이라 생각하는데, 배열을 넣은 이유가 있을까요??
|
|
||
| TimeTableRetrieveDto result = meetingRetrieveService.getTimeTable(1L, 1L); | ||
|
|
||
| assertThat(expected).isEqualTo(result); |
There was a problem hiding this comment.
P4
테스트 결과가 나오는 순서와 일치시키기 위해 result , expected 순이면 좋을 것 같습니다!
✒️ 관련 이슈번호
Key Changes 🔑
To Reviewers 📢