-
Notifications
You must be signed in to change notification settings - Fork 3
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
[BE] refactor: Club 매핑 테이블 엔티티 연관 관계 개선 #165
Conversation
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 List<String> collectOverviewPetImages(Club club) { | ||
Map<Long, List<Pet>> groupPetsByMemberId = clubPetRepository.findAllByClubId(club.getId()).stream() | ||
.map(ClubPet::getPet) | ||
.collect(Collectors.groupingBy(pet -> pet.getMember().getId())); |
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.
굳굳 이렇게 해결됐구나..!
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.
연관관계 개선 고생 많았습니다!!!
이번에도 사소한 네이밍이랑 테스트 코드 위주로 리뷰해봤어요.
저번 PR에도 리뷰 있으니까 참고해주세요~!!
중요한 부분은 아니라서 approve 하겠습니다!
@ManyToOne(optional = false, fetch = FetchType.LAZY) | ||
@JoinColumn(name = "club_member_id", nullable = false) | ||
private ClubMember clubMember; | ||
@JoinColumn(name = "club_id", nullable = false) | ||
private Club club; | ||
|
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.
👍
ClubMember clubMember = ClubMember.builder() | ||
.member(member) | ||
.club(club) | ||
.build(); | ||
|
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.
clubMember는 member, club 순으로 생성하고
clubPet은 club, pet 순으로 생성하고 있네요.
빌더 패턴을 쓴다고 하더라도, 하나로 통일시키면 좋을 것 같아요!
"https://image.com"); | ||
|
||
protected Club saveNewClub() { | ||
protected Club getSavedClub(Set<Gender> genders, Set<SizeType> sizes) { |
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.
protected Club getSavedClub(Set<Gender> genders, Set<SizeType> sizes) { | |
protected Club createClub(Set<Gender> genders, Set<SizeType> sizes) { |
메서드명은 get으로 되어 있지만 내부적으로는 생성도 하고 있네요.
create라는 이름은 어떨까요?
|
||
List<FindSearchingClubResponse> responses = clubQueryService.findSearching(request); | ||
|
||
assertThat(responses.isEmpty()).isTrue(); |
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.
assertThat(responses.isEmpty()).isTrue(); | |
assertThat(responses).isEmpty(); |
private List<String> collectOverviewPetImages(Club club) { | ||
Map<Long, List<Pet>> groupPetsByMemberId = clubPetRepository.findAllByClubId(club.getId()).stream() | ||
.map(ClubPet::getPet) | ||
.collect(Collectors.groupingBy(pet -> pet.getMember().getId())); | ||
|
||
return groupPetsByMemberId.values().stream() | ||
.map(petList -> petList.get(0).getImageUrl().getValue()) | ||
.collect(toList()); | ||
} |
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 List<String> collectOverviewPetImages(Club club) { | |
Map<Long, List<Pet>> groupPetsByMemberId = clubPetRepository.findAllByClubId(club.getId()).stream() | |
.map(ClubPet::getPet) | |
.collect(Collectors.groupingBy(pet -> pet.getMember().getId())); | |
return groupPetsByMemberId.values().stream() | |
.map(petList -> petList.get(0).getImageUrl().getValue()) | |
.collect(toList()); | |
} | |
private List<String> findPetImageUrls(Club club) { | |
Map<Long, List<Pet>> groupPetsByMemberId = clubPetRepository.findAllByClubId(club.getId()).stream() | |
.map(ClubPet::getPet) | |
.collect(groupingBy(pet -> pet.getMember().getId())); | |
return groupPetsByMemberId.values().stream() | |
.map(petList -> petList.get(0).getImageUrl().getValue()) | |
.toList(); | |
} |
- 메서드 이름이 더 직관적이면 좋겠어요. overview는 전체 펫의 요약이라는 느낌이 들어서요.
- 불변 리스트로 리턴하면 더 좋겠네요!
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.
위브 고생 정말 많으셨어요!
밤새 작성한 코드 다시 엎기 쉽지 않았을텐데 수고하셨습니다!
참고로 동적 쿼리 생성 시 복잡한 JpaSpecification (Criteria) 보다는 QueryDSL이나 Jooq 와 같은 프레임워크가 권장된다고 합니다~
관련 내용 더 찾아보시면 개선할 부분이 생길 것 같아요!
이슈
개발 사항
리뷰 요청 사항 (없으면 삭제해 주세요)
전달 사항 (없으면 삭제해 주세요)