[FEAT] 서재 기능 업데이트 구현 #356
[FEAT] 서재 기능 업데이트 구현 #356
Conversation
github-actions
bot
requested review from
EunjeongHeo,
GiJungPark,
Kim-TaeUk and
sansan20535
| Novel novel = userNovel.getNovel(); | ||
|
|
||
| List<String> attractivePoints = userNovel.getUserNovelAttractivePoints().stream() | ||
| .map(UserNovelAttractivePoint::getAttractivePoint) |
There was a problem hiding this comment.
p5; 오호? map도 중복이 가능하군요..! 하나 알아갑니다 : )
There was a problem hiding this comment.
map... 중복이 안되지 않나요?
|
|
||
| private <T> void applyFilters(JPAQuery<T> queryBuilder, Boolean isInterest, List<String> readStatuses, | ||
| List<String> attractivePoints, Float novelRating, String query) { | ||
| Optional.ofNullable(isInterest) |
There was a problem hiding this comment.
p5; null 값 검사를 Optional을 사용하는 방법과 queryBuilder를 사용하는 방법이 있었네요..! 저는 개인적으로 필터 부분을 따로 분리한 게 좋은 것 같습니다!
| private final GenreRepository genreRepository; | ||
| private final FeedRepository feedRepository; | ||
|
|
||
| public static final String SORT_TYPE_OLDEST = "OLDEST"; |
There was a problem hiding this comment.
p4; SORT TYPE을 enum으로 관리해도 좋을 것 같습니다!
현재 피드소소 사일로 관련해서도 정렬 기준을 작성했는데, 이러한 기준을 enum으로 모아 관리하면 직관적이고 유지보수도 쉬워지지 않을까 생각이 들었습니다!
| String startDate, | ||
| String endDate, |
There was a problem hiding this comment.
p5; startDate 랑 endDate가 LocalDate가 아닌 String인 이유가 있을지 궁금합니다!! 다른 Dto인 UserNovelGetResponse 레코드를 보면 같은 이름인 startDate, endDate 필드가 LocalDate로 되어 있어서요!
(추가로, UserNovelGetResponse랑 UserNovelAndNovelGetResponse가 null 여부에 따른 객체 생성 방식에 대한 컨벤션도 조금 다른 것 같은데 추후에 방식 통일시켜주면 좋을 것 같아요!)
There was a problem hiding this comment.
오옷 놓쳤네요. 찾아주셔서 감사합니다~~
리뷰 반영 위한 새로운 pr에서 적용하도록 하겠습니다.
There was a problem hiding this comment.
+) null 여부에 따른 객체 생성 방식의 경우, readStatus가 null인 경우를 처리하는 방식은 통일되어있습니다!
다만 UserNovelGetResponse는 평가가 없을 경우에도 호출될 수 있는 함수라, userNovel 자체가 null인 경우를 처리하느라 다르게 보이는 것 같습니다.
혹시 다른 통일 방식이 있을까요~?
There was a problem hiding this comment.
당시에 어떤 컨벤션을 의미했던 건가 싶어서 다시 코드를 확인해보니, 제가 로직 자체에 대한 의도 파악이 부족했던 것 같습니다!
다른 제안 사항은 없을 것 같아요~! 설명 감사합니다!
| List<UserNovelAndNovelGetResponse> userNovelAndNovelGetResponses = userNovelsByNoOffsetPagination.stream() | ||
| .map(UserNovelAndNovelGetResponse::of) | ||
| boolean isOwner = visitor.getUserId().equals(ownerId); | ||
| boolean isAscending = sortType.equalsIgnoreCase(SORT_TYPE_OLDEST); |
There was a problem hiding this comment.
p3: SORT_TYPE_OLDEST.equalsIgnoreCase(sortType)처럼 상수를 기준으로 문자열 비교 메서드를 사용하게는게 NPE 발생 위험성을 제거할 수 있을것 같습니다!
There was a problem hiding this comment.
너무 좋습니다! 새로운 pr에 반영해두도록 하겠습니다~!
Related Issue
Key Changes
서재소소(2차 사일로) 관련 기능입니다.
서재(유저 보관함)가 업데이트 되어 기능을 다시 구현하였습니다.
isHidden을 제외한 모든 피드 응답isPublic이 true이고,isSpoiler가 false여야 하는 추가사항To Reviewers
References