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

#220 [Refactor] 제안서 상세보기 API Controller, Service 분리 #228

Merged
merged 13 commits into from
Feb 4, 2024

Conversation

hellozo0
Copy link
Member

@hellozo0 hellozo0 commented Jan 31, 2024

관련 이슈번호

해결하는 데 얼마나 걸렸나요? (예상 작업 시간 / 실제 작업 시간)

  • 1h / 1h 20m

해결하려는 문제가 무엇인가요?

  • 제안서 상세보기 API Controller, Service 분리

어떻게 해결했나요?

  • Body 안에 두가지 Response 객체로 쌓여서 나오는데, 크게 객체 단위로 함수를 나눠서 진행을 하다가 / Service 파일이 달라져야하는 부분들만 함수를 더 쪼개서 작업했습니다~!
스크린샷 2024-01-31 오후 9 08 30 스크린샷 2024-01-31 오후 9 08 49

@hellozo0 hellozo0 requested review from KWY0218 and pkl0912 January 31, 2024 12:14
@hellozo0 hellozo0 self-assigned this Jan 31, 2024
Copy link
Member

@KWY0218 KWY0218 left a comment

Choose a reason for hiding this comment

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

작업하시느라 고생하셨습니다!
코맨트 확인해주세요!

Comment on lines 69 to 72
List<Boolean> preferOfferConditionBooleanList = Arrays.stream(OfferCondition.values()).map(condition -> {
if (offerConditionList.contains(condition)) return true;
else return false;
}).collect(Collectors.toList());
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
List<Boolean> preferOfferConditionBooleanList = Arrays.stream(OfferCondition.values()).map(condition -> {
if (offerConditionList.contains(condition)) return true;
else return false;
}).collect(Collectors.toList());
List<Boolean> preferOfferConditionBooleanList = Arrays.stream(OfferCondition.values()).map(offerConditionList::contains).collect(Collectors.toList());

p2
contains 자체가 boolean을 반환하기 때문에 함다를 사용하여 위 코드 처럼 코드를 줄일 수 있을 것 같습니다

return new DesignerInfoOpenChatDto(designer.getKakaoOpenChatUrl(),designer.getProfileImgUrl(), designer.getHairShop().getName(), designer.getName(), designer.getIntroduction());
List<String> dayOfWeekList = getDayOfWeekList(designerId);

DesignerInfoResponse designerInfoResponse = new DesignerInfoResponse(designer.getProfileImgUrl(), designer.getHairShop().getName(), designer.getName(), designer.getPortfolio().getInstagramUrl(), designer.getPortfolio().getNaverPlaceUrl(), designer.getIntroduction(), designer.getGender().getValue(), dayOfWeekList, designer.getHairShop().getAddress(), designer.getHairShop().getDetailAddress());
Copy link
Member

Choose a reason for hiding this comment

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

p4
해당 객체와 같이 생성자가 많은 dto인 경우에는 builder를 사용하는 것이 좋다고 생각합니다..!
어떻게 생각하시나요?

Copy link
Member Author

Choose a reason for hiding this comment

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

@KWY0218 저도 그부분 고민을 했는데 파라미터 값들이 많아서 Builder 사용이 좋을것 같네요! :) 🫡

Copy link
Member

@KWY0218 KWY0218 left a comment

Choose a reason for hiding this comment

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

작업하시느라 고생하셨습니다!!

@hellozo0 hellozo0 merged commit aa6bcc7 into develop Feb 4, 2024
1 check passed
@hellozo0 hellozo0 deleted the refactor/#220 branch February 4, 2024 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[refactor] [모델 뷰] 디자이너 제안 상세보기 API Controller, Service 분리하기
2 participants