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.

Sign up for GitHub

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] 빵집 상세보기 리팩터링 #145

Merged
merged 4 commits into from
Aug 14, 2023
Merged

[FIX] 빵집 상세보기 리팩터링 #145

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
85ec9f4
[FIX] BakeryMapper를 사용하여 toBakeryDetailResponseDTO를 통한 리팩토링 구현해라
sung-silver Aug 14, 2023
7aba394
[FIX] MenuMapper를 작성하고 BakeryService 내 getBakeryDetail 로직에 적용해라
sung-silver Aug 14, 2023
2f2a4ef
[FIX] 매개변수 전달을 위한 단순 변수 선언으로 생각되는 로직 생략해라
sung-silver Aug 14, 2023
a3f906d
[FIX] 코드리뷰 반영
sung-silver Aug 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion api/src/main/java/com/org/gunbbang/controller/DTO/response/BakeryDetailResponseDTO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public class BakeryDetailResponseDTO extends BaseBakeryResponseDTO {
private String homepageUrl;
private String instagramUrl;
private String address;
private String openingTime;
private String openingHours;
private String closedDay;
private String phoneNumber;
private List<MenuResponseDTO> menuList;
Expand Down
39 changes: 10 additions & 29 deletions api/src/main/java/com/org/gunbbang/service/BakeryService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,14 @@ public BakeryDetailResponseDTO getBakeryDetail(Long memberId, Long bakeryId) {
BreadTypeResponseDTO breadType = getBreadType(bakery);
Copy link
Collaborator

Choose a reason for hiding this comment

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

BreadTypeResponseDTO로 변환하는 매퍼 만들었던걸로 기억하는데 있으면 매퍼로 처리하는것도 괜찮아 보입니다~

Copy link
Member Author

Choose a reason for hiding this comment

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

매퍼로 동일하게 처리하고 리팩토링하면서 다른 곳에서 쓰이면 같이 수정하겠습니다~

Copy link
Collaborator

Choose a reason for hiding this comment

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

제가 이미 쓰긴합니다 하암ㅋㅋ 매퍼 코드 있으니 함 생각해보세요~

boolean isBookMarked = isBookMarked(memberId, bakeryId);
List<MenuResponseDTO> menuList = new ArrayList<>();
String address =
bakery.getState()
+ " "
Copy link
Collaborator

Choose a reason for hiding this comment

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

요거 띄어쓰기 BLANK_SPACE 이런 식으로 상수화 처리 어떠실까욜

Copy link
Collaborator

Choose a reason for hiding this comment

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

추가로 address 가져오는 부분 함수로 빼는건 어떠실까욜 매개변수가 너무 많을까요??

Copy link
Member Author

Choose a reason for hiding this comment

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

오옹 상수화는 생각도 못했네요 좋습니다! address 가져오는 부분도 함수로 빼는거 좋습니다!

+ bakery.getCity()
+ " "
+ bakery.getTown()
+ " "
+ bakery.getAddressRest();

for (Menu menu : bakeryMenu) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

메뉴 리스트 가져오는 것도 함수로 빼도 괜찮을 것 같습니다~

Copy link
Member Author

Choose a reason for hiding this comment

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

가장 최근 commit에 이친구도 Mapper로 처리했습니다 !!

Copy link
Collaborator

Choose a reason for hiding this comment

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

굿 좋습니다~

menuList.add(
Expand All @@ -92,35 +100,8 @@ public BakeryDetailResponseDTO getBakeryDetail(Long memberId, Long bakeryId) {
.build());
}

return BakeryDetailResponseDTO.builder()
.bakeryId(bakery.getBakeryId())
.bakeryName(bakery.getBakeryName())
.bakeryPicture(bakery.getBakeryPicture())
.isHACCP(bakery.getIsHACCP())
.isVegan(bakery.getIsVegan())
.isNonGMO(bakery.getIsNonGMO())
.breadType(breadType)
.firstNearStation(bakery.getFirstNearStation())
.secondNearStation(bakery.getSecondNearStation())
.isBookMarked(isBookMarked)
.bookMarkCount(bakery.getBookMarkCount())
.reviewCount(bakery.getReviewCount())
.mapUrl(bakery.getMapUrl())
.homepageUrl(bakery.getHomepageUrl())
.instagramUrl(bakery.getInstagramUrl())
.address(
bakery.getState()
+ " "
+ bakery.getCity()
+ " "
+ bakery.getTown()
+ " "
+ bakery.getAddressRest())
.openingTime(bakery.getOpeningHours())
.closedDay(bakery.getClosedDay())
.phoneNumber(bakery.getPhoneNumber())
.menuList(menuList)
.build();
return BakeryMapper.INSTANCE.toBakeryDetailResponseDTO(
Copy link
Collaborator

Choose a reason for hiding this comment

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

휴우 깔끔하네요~ 마음의 안정

bakery, address, breadType, isBookMarked, menuList);
}

public List<BestBakeryListResponseDTO> getBestBakeries(Long memberId) {
Expand Down
15 changes: 11 additions & 4 deletions api/src/main/java/com/org/gunbbang/util/mapper/BakeryMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
package com.org.gunbbang.util.mapper;

import com.org.gunbbang.controller.DTO.response.BakeryListResponseDTO;
import com.org.gunbbang.controller.DTO.response.BakerySearchResponseDTO;
import com.org.gunbbang.controller.DTO.response.BestBakeryListResponseDTO;
import com.org.gunbbang.controller.DTO.response.BreadTypeResponseDTO;
import com.org.gunbbang.controller.DTO.response.*;
import com.org.gunbbang.entity.Bakery;
import java.util.List;
import org.mapstruct.Mapper;
Expand All @@ -24,4 +21,14 @@ BakerySearchResponseDTO toBakerySearchResponseDTO(
int resultCount, List<BakeryListResponseDTO> bakeryList);

List<BestBakeryListResponseDTO> toBestBakeryListResponseDTO(List<Bakery> bakeries);

@Mapping(source = "bakery.isHACCP", target = "isHACCP")
@Mapping(source = "bakery.isVegan", target = "isVegan")
@Mapping(source = "bakery.isNonGMO", target = "isNonGMO")
BakeryDetailResponseDTO toBakeryDetailResponseDTO(
Bakery bakery,
String address,
BreadTypeResponseDTO breadType,
boolean isBookMarked,
List<MenuResponseDTO> menuList);
}