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

[FIX] 건빵집 상세보기에 길찾기 url 값 추가해라 #134

Merged
merged 5 commits into from
Aug 11, 2023

Conversation

sung-silver
Copy link
Member

@sung-silver sung-silver commented Aug 11, 2023

🍞 PR 타입

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

🍞 반영 브랜치

🍞 변경 사항

  • bakery 엔티티에 mapUrl 필드 추가했습니다
  • bakeryDetailResponseDTO에 mapUrl 필드를 추가하고 사용되는 service, controller 코드 수정했습니다
  • log 파일 gitignore에 추가하고 캐시를 지웠습니다
  • h2 database script map_url 추가에 따라 적절하게 수정했습니다

🍞 테스트 결과

image
image

🍞 To Reviewer

log/
Copy link
Collaborator

Choose a reason for hiding this comment

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

수정해줘서 고마우이,,,, 🥹🥹🥹🥹

@@ -22,6 +22,7 @@ create table bakery
city varchar(255) not null,
closed_day varchar(255),
first_near_station varchar(255),
map_url varchar(255) not null,
Copy link
Collaborator

Choose a reason for hiding this comment

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

환경별로 url 길이 기준이 조금씩 다른데 그래도 엄청 길긴하네요,,, 2000으로 하기엔 무리일 것 같고 보통 길찾기 url 길이가 어느정도 되는지 찾아본 다음에 늘리는건 어떠신가요???

https://zetawiki.com/wiki/URL%EC%9D%98_%EC%B5%9C%EB%8C%80_%EA%B8%B8%EC%9D%B4

Copy link
Member Author

Choose a reason for hiding this comment

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

오.. 이건 생각 못했네요!!! 감사합니다 검색해보니 varchar가 최대 21,845라는 게시글이 많은데, 일단 2000으로 수정하는건 어떨까요??

Copy link
Collaborator

Choose a reason for hiding this comment

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

좋습니다~ url 길이가 생각보다 짧다면 거기서 줄여도 될 것 같아용

Copy link
Member Author

Choose a reason for hiding this comment

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

  • 논의 결과: 기획이 더미 다 쌓으면 URL 최대 길이인 것을 고려하여 정하기

@seunghaLim
Copy link
Collaborator

변동 안된 코드에는 리뷰가 안남겨져서ㅠㅠ 여기에다가라도 남깁니다

  1. sortOption 변수값 가져오는데 삼항연산자 쓰셨는데 삼항연산자를 써야하는지 안쓰는게 좋은지 의견이 분분하더라구요 아래 아티클 함 읽어보시고 if문으로 뺄지 함수로뺄지 삼항연산자 그대로쓸지 고민하셔도 좋을듯 합니다 일단 남겨주신 코드는 크게 가독성이 떨어지진 않는다고 느꼈어용

https://kwaksh2319.tistory.com/627

@seunghaLim
Copy link
Collaborator

  1. BreadType / NutrientType / List 각각을 DTO로 변환할 때 매퍼 사용해보시는건 어떨까요? 제가 저번에 pr 날렸던 것처럼요

매퍼를 쓰면 1) 해당 변환 로직을 한 곳에서 모아볼 수 있고(지금은 서비스 클래스마다 분산되어있음) 2) 코드 길이도 줄어들어서 좋을 것 같다고 생각했습니다 다른 의견 있으면 코멘트 plz~

@seunghaLim
Copy link
Collaborator

seunghaLim commented Aug 11, 2023

  1. 이부분 함수로 빼는건 어떠신가용
for (Bakery bakery : bakeryList) {
        breadType = getBreadType(bakery);
        isBookMarked = isBookMarked(memberId, bakery.getBakeryId());
        BakeryListResponseDTO bakeryListResponseDto =
            getBakeryResponseDTO(bakery, isBookMarked, breadType);
        responseDtoList.add(bakeryListResponseDto);
      }
      return responseDtoList;

@seunghaLim
Copy link
Collaborator

일단 머지하시고 리뷰남긴건 천천히생각해보신 뒤에 나중에 수정하셔도 될 것 같습니동~

@sung-silver
Copy link
Member Author

아래 comment로 남겨주신 부분은 리팩토링으로 빼서 다시 PR 날리겠습니다 감사합니다!!

@sung-silver sung-silver merged commit 83f3687 into dev Aug 11, 2023
1 check passed
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.

[FIX] 빵집 상세보기에 네이버 지도 길찾기 URL 추가
2 participants