-
Notifications
You must be signed in to change notification settings - Fork 7
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
좋아요 기능 구현 #680
좋아요 기능 구현 #680
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.
짱수 수고했어요 ~!!!
굿굿!! 최고다 짱수 ~!
코멘트 간단하게 남겨뒀어요 확인부탁드려요 ~
if (isLike(template, member)) { | ||
return; | ||
} | ||
Likes likes = template.like(member); |
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.
해당 메서드를 사용하는 부분에서 어떤 로직을 하는 메서드인지 알기 어려운 것 같아요.
이름을 더 명확하게 변경하면 좋을 것 같아요 ! 예를 들어 createLike?
그리고 like 생성을 왜 template 도메인이 가지도록 한건지 궁금합니다.
굳이 템플릿 도메인의 역할이라고 생각되지 않아서 저는 LikeService에서 new Likes(null, template, member);
하는 로직이 더 선호됩니다. 어떻게 해서 도메인이 가지도록 설정한 건지 궁금합니다 ~
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.
Likes
도메인 생성의 책임을 특정 도메인의 책임으로 준 이유는 객체의 책임 설계를 할 때 DB 를 사용한다는 가정을 벗어나서 설계했기 때문이에요.
(꼭 그래야 한다는 것이 아니라, 제가 그렇게 생각하는 것에 익숙해져서.. 제 생각의 흐름을 설명드리기 위함입니다!
저는 레벨 2부터 계속해서 웹 / DB 사용 등의 조건에 종속적이지 않은 Service layer 코드를 작성하는 것을 이상적이라 생각하고 있어요)
우선 이 부분은 new Likes()
로 반영을 해 두겠습니다!
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.
Likes 도메인 생성의 책임을 특정 도메인의 책임으로 준 이유는 객체의 책임 설계를 할 때 DB 를 사용한다는 가정을 벗어나서 설계했기 때문이에요.
그럼 짱수는 특정 도메인의 책임으로 안주고 서비스에서 객체를 생성하는 방식은 DB를 사용한다를 고려한 건가요?
저는 서비스에서 new Likes() 하는 코드 또한 DB를 고려하지 않은 설계라고 생각해요! DB를 고려한, 즉 짱수가 말하는 DB를 사용한다는 가정을 가지고 설계한 코드는 무엇인지 궁금합니다 ~ ! 지금 당장 이야기 안해줘도 됩니다 ! 단순 궁금증이니 나중에 이야기해줘도 돼요~
우선 이 부분은 new Likes() 로 반영을 해 두겠습니다!
굿 ~!
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.
DB 를 사용한다는 것을 인지한 상태로 책임을 설계한다면 테이블이 먼저 나왔을거에요.
다대다 연관관계이니 자연스럽게 Likes
도메인이 존재했을 것 같습니다.
Likes
도메인이 존재하니 Template.like()
메서드 대신 service 계층에서 해당 도메인을 생성했을 것 같고요.
반대로 DB 를 사용한다는 생각을 하지 않는다면 굳이 Like
라는 도메인이 객체의 책임을 고려할 때 부터 만들어 질 필요가 없다고 생각해요.
우리가 다대다 테이블을 직접 관리하기로 결정 했기 때문에 해당 메서드의 반환값으로 Likes
도메인이 반환 되어서 LikesRepository
에 저장이 되는 코드로 변했을거에요.
만약 우리가 @ManyToMany
로 다대다 연관관계를 관리했다면 service 계층의 코드 변화도 없었지 않을까 생각합니다!
backend/src/main/java/codezap/template/service/facade/MemberTemplateApplicationService.java
Outdated
Show resolved
Hide resolved
backend/src/main/java/codezap/template/service/facade/TemplateApplicationService.java
Outdated
Show resolved
Hide resolved
backend/src/main/java/codezap/template/service/facade/TemplateApplicationService.java
Outdated
Show resolved
Hide resolved
backend/src/test/java/codezap/template/service/facade/TemplateApplicationServiceTest.java
Outdated
Show resolved
Hide resolved
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.
짱수 미션과 병행하느라 바빴을텐데 정말 고생했어요~~!👍👍👍
사소해 보일 수도 있지만,
저는 좋아요라는 기능을 명세한다면 "사용자가 템플릿을 좋아요 표시한다"가 될 것 같아요.
영어로는 "Member likes template" 이 되겠죠.
그렇다면 관계의 방향이 멤버에서 템플릿으로 가는 방향이라고 생각해요.
Member → Template
그래서 member와 template 객체 둘을 파라미터로 하는 메서드들의 파라미터 순서가 member, template 순이었으면 좋겠어요.
LikesService.isLike
LikesRepository.existsBy...
LikesRepository.deleteBy...
LikesJpaRepositoryTest
@DisplayName("템플릿과 멤버 정보로 좋아요 조회 테스트")
→멤버 정보와 템플릿으로
@DisplayName("템플릿과 멤버로 좋아요 삭제 테스트")
→ 상동
짱수 생각은 어떤가요?
likesRepository.save(likes); | ||
} | ||
|
||
public Boolean isLike(Template template, Member member) { |
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.
public Boolean isLike(Template template, Member member) { | |
public Boolean isLiked(Template template, Member member) { |
@GetMapping("/{id}") | ||
public ResponseEntity<FindTemplateResponse> getTemplateById( | ||
@PathVariable Long id | ||
) { | ||
return ResponseEntity.ok(memberTemplateApplicationService.getTemplateById(id)); | ||
} | ||
|
||
@GetMapping("/{id}/login") |
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.
로그인을 하지 않았을 경우에는 서버든 프론트든 예외를 처리하는 게 낫지 않을까요?
로그인 없이 좋아요를 누를 경우 alert 후 로그인 페이지로 이동하는 선택지를 주는 게 일반적인 상용 서비스의 흐름이니까요.
backend/src/main/java/codezap/template/service/facade/TemplateApplicationService.java
Outdated
Show resolved
Hide resolved
import codezap.member.domain.Member; | ||
import codezap.template.domain.Template; | ||
|
||
//기존의 Fake repo 사용 코드가 터지지 않도록 임시로 구현하는 코드 |
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.
아잇, 이거 지우고 올릴랬는데 ㅠ
바로 반영 했습니다~!
backend/src/test/java/codezap/likes/service/LikesServiceTest.java
Outdated
Show resolved
Hide resolved
backend/src/test/java/codezap/likes/service/LikesServiceTest.java
Outdated
Show resolved
Hide resolved
backend/src/test/java/codezap/template/service/facade/MemberTemplateApplicationServiceTest.java
Outdated
Show resolved
Hide resolved
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.
안녕하세요 짱수 🔥
리뷰 반영 수고했어요 쵝오 👍
아주 열심히 구현하신 게 보입니다 👀
아직 함께 이야기 나눠보고 싶은 부분이 있어요 조금 더 얘기 나눠봐요 퐈이팅 😁
@FunctionalInterface | ||
private interface LikePredicate { | ||
|
||
boolean isLike(Template template); | ||
} | ||
|
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.
따로 파일을 분리하지 않고 TemplateApplicationService에 둔 이유가 있을까요?
좋아요 전략이 like 패키지가 있는데도 불구하고 TemplateApplicationService에 있는 게 조금 어색해보여요
파일을 분리하여 좋아요 전략은 좋아요 도메인이 관리할 수 있도록 like 하위에 두는 것이 향후에 관련 전략이 변경되었을 때 관리하기 편할 것 같은데 짱수는 어떻게 생각하시나요? 다른 이유가 있었다면 듣고 싶어요 👀
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 makeTemplatesResponse(templates, (template) -> false); | ||
} | ||
|
||
public FindAllTemplatesResponse findAllByWithMember(Long memberId, String keyword, Long categoryId, | ||
List<Long> tagIds, Pageable pageable, Member loginMember | ||
) { | ||
Page<Template> templates = templateService.findAll(memberId, keyword, categoryId, tagIds, pageable); | ||
return makeTemplatesResponse(templates, (template -> likesService.isLike(template, loginMember))); |
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.
짱수~ 코멘트 보고 다시 답변해요
제가 전략패턴을 오해하고 있는 걸 수도 있으니 서로 편하게 이야기 해봅시다.
짱수의 의도는 전략 패턴을 통해 로그인했을 경우, 안했을 경우 좋아요 조회 방식을 적용하고 싶은 것 같아요.
다만 현재 코드상으로 그게 잘 보이지는 않는 것 같아요
제가 생각하기에 어색하다고 느껴지는 이유가
- 현재 짱수가 말한 전략 패턴이 사용된 건지 한눈에 파악하기 어려운 것 같아요.
- 좋아요 전략이 조금만 변경이 되어도 Template 파사드가 변경되어버려요. (전략의 책임이 TemplateApplicationSerivce에 있는 것 같아요)
이전에 이야기가 잠깐 나왔던 것처럼 만약 비회원도 좋아요 기능을 사용하게 된다면 어떨까요?
(비회원의 IP를 사용해 좋아요 기능을 구현한다던지, 브라우저 로컬 스토리지에 좋아요를 저장한다던지 여러 방법이 있겠지만 백엔드가 처리한다고 가정해봅시다)
비회원의 좋아요 전략이 변경되면서 위의 현재 (template) -> false
가 변경되면서 Template 파사드가 변경되어버려요.
제가 이해한 전략 패턴은 클라이언트 (여기는 전략을 사용하는 TemplateApplication) 가 내부 로직까지는 몰라도 된다고 생각해요. (제가 잘못 이해했다면 말해주세용)
따라서 LikePredicate 인터페이스를 작성해주신만큼 각 전략에 대한 구현체를 만들면 어떨까요?
(예시 - 이렇게 하라는 거 아님)
@RequiredArgsConstructor
public class LoginMemberLikeStrategy implements LikeStrategy {
private final LikesService likesService;
public boolean isLike(Template template, Member member) {
return likesService.isLike(template, member);
}
}
// TemplateApplicationService.java
public FindAllTemplatesResponse findAllByWithMember(Long memberId, String keyword, Long categoryId,
List<Long> tagIds, Pageable pageable, Member loginMember
) {
Page<Template> templates = templateService.findAll(memberId, keyword, categoryId, tagIds, pageable);
return makeTemplatesResponse(templates, loginMember, new LoginMemberLikeStrategy(likesService));
}
- ; 추가
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.
Wow 짱수 완전 수고했어요
이 PR에 좋아요를 바칩니다~!!!!!!!!!!!
쵝5쵝6쵝7쵝8
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.
고생많았어요~!
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.
변경사항 확인했습니다.
고생하셨습니다 👍수~~
⚡️ 관련 이슈
#668
📍주요 변경 사항
참고 사항
Likes.likesCount 가상 필드에 대해
@formula 는 실제 DB 에 컬럼으로 존재하지 않는 값을 필드로 매핑하기 위해 사용합니다.
"좋아요 많은 순으로 정렬" 기능을 구현하기 위해 추가되었습니다.
출처 : 벨로그
이곳에서도 성능 문제가 발생할 가능성이 존재하며, "좋아요 순 정렬" 의 구현 방법 자체에 대해 더 논의가 필요할 수 있겠습니다.