Skip to content
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

[Refactor] 베스트 컨텐츠 조회 api 리팩토링 및 북마크 여부 제외 #135

Merged
merged 3 commits into from
Aug 12, 2023

Conversation

seunghaLim
Copy link
Collaborator

🍞 PR 타입

  • 기능 추가
  • 기능 수정
  • 기능 삭제
  • 버그 수정
  • 의존성, 환경 변수, 빌드 관련 코드 업데이트

🍞 반영 브랜치

refactor/#128 -> dev

🍞 변경 사항

  • 베스트 건빵집 조회 api 리팩토링
  • 베스트 리뷰 조회 api 리팩토링
  • 북마크 여부 필드 isBookMarked를 건빵집 상세보기 api 제외한 다른 api 응답 필드에서 제외

🍞 테스트 결과

🍞 To Reviewer

  • 베스트 건빵집 및 베스트 리뷰 조회 시 사용되는 service단 메서드를 깔끔하게 정리하려고 했으나,, 잘 된 건지 잘 모르겠네요 확인 부탁드립니다
  • in절에 빈 배열이 올 경우 조회가 아무것도 되지 않는 이슈가 있었습니다 해결책으로는
  1. 배열에 -1L과 같은 절대 ture가 나올 수 없는 값을 넣거나
  2. 객체로 조회하되 서브쿼리를 사용하거나
    두 가지 방안이 나왔었는데 저는 1)을 사용했습니다(서브쿼리 성능 문제 때문에 그런건데 사실 해당 쿼리에 사용할 서브 쿼리가 join하는 것도 없어서 크게 문제될 건 없어보이긴 했습니다)

횩시 다른 의견 있으실지 코멘트 부탁드립니다

Copy link
Member

@sung-silver sung-silver left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

함수 분리는 제가 봤을 때 잘 된거같습니다! Mapper 이용하는거는 저도 처음봐서 이 부분에 대해서는 공부해보고 리팩토링 때 적용시켜보도록 하겠습니다 수고하셨습니다!

@@ -129,81 +120,67 @@ public BakeryDetailResponseDTO getBakeryDetail(Long memberId, Long bakeryId) {
.build();
}

public List<BestBakeryListResponseDTO> getBestBakeries() {
Long memberId = SecurityUtil.getLoginMemberId();
public List<BestBakeryListResponseDTO> getBestBakeries(Long memberId) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Best 건빵집 조회가 확실히 복잡하긴하네유... 10개가 안될 경우 계속 계속 채우는 느낌이라 2차 릴리즈 때는 확실히 알고리즘 짜는게 좋아보입니다 랜덤으로 던져주는 것도 한계가 있어보여서...!!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

동의합니더,,

@seunghaLim seunghaLim merged commit 3563aae into dev Aug 12, 2023
1 check passed
@seunghaLim seunghaLim deleted the refactor/#128 branch February 17, 2024 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants