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

2단계 - 리팩터링(메뉴) #233

Open
wants to merge 6 commits into
base: eleeje97
Choose a base branch
from
Open

Conversation

eleeje97
Copy link

안녕하세요.
2단계 미션 리뷰 부탁드립니다.

몇가지 이슈가 있는데, pr 올리는 것이 너무 늦어져 우선 올리겠습니다!

  • Menu 도메인이 DisplayedName, Price VO를 갖는데, @Embedded 어노테이션을 사용해도 적용되지 않습니다.
  • Menu를 생성하기 위한 DTO를 따로 만들었는데, 이 때 ProductId를 입력받고 그 id로 product를 찾아 MenuProduct에 주입하여 Menu를 생성합니다. 이 부분에 대한 테스트코드 작성에 어려움이 있습니다.

감사합니다.

@eleeje97 eleeje97 changed the title Step2 2단계 - 리팩터링(메뉴) Sep 21, 2023
@sah3122
Copy link

sah3122 commented Sep 22, 2023

안녕하세요 다은님 😄

Menu를 생성하기 위한 DTO를 따로 만들었는데, 이 때 ProductId를 입력받고 그 id로 product를 찾아 MenuProduct에 주입하여 Menu를 생성합니다. 이 부분에 대한 테스트코드 작성에 어려움이 있습니다.

해당 질문을 제대로 이해하지 못했는데 조금더 상세히 설명 부탁드립니다 !

Copy link

@sah3122 sah3122 left a comment

Choose a reason for hiding this comment

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

메뉴 리팩터링 잘해주셨습니다 😄
몇가지 코멘트 남겨두었는데 확인 부탁드려요 🙏

import java.math.BigDecimal;
import java.util.Objects;

public class MenuPrice {
Copy link

Choose a reason for hiding this comment

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

@embeddable 어노테이션을 정의해주어야 할것 같아요 🙏

@Embeddable
public class MenuPrice {

아래 문서를 참고해보세요 😄
https://velog.io/@seongwon97/Spring-Boot-JPA-Embedded-Embeddable

foreignKey = @ForeignKey(name = "fk_menu_product_to_product")
name = "product_id",
columnDefinition = "binary(16)",
foreignKey = @ForeignKey(name = "fk_menu_product_to_product")
)
private Product product;
Copy link

Choose a reason for hiding this comment

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

Product 엔티티를 직접참조로 유지 하는것 보단 간접 참조로 변경해보는건 어떨까요 ?
강의에서 배운 Aggregate Pattern을 참고해보세요 😄


List<MenuProduct> menuProducts = request.getMenuProducts().stream()
.map(menuProduct -> {
Product product = productRepository.findById(menuProduct.getProductId())
Copy link

Choose a reason for hiding this comment

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

Menu Service 에서 Product 도메인과의 결합을 제거해볼수 있을까요 ?
외부 도메인과 협력을 해야하는 로직들을 DomainService로 정의해볼수 있을것 같습니다 😄

}
}

private boolean isMenuPriceGreaterThanSumOfProductPrice() {
return price.compareTo(getSumOfProductPrice()) > 0;
Copy link

Choose a reason for hiding this comment

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

Product 엔티티를 간접 참조로 변경한다면 이와 같은 기능을 어떻게 구현할지 고민해보세요 😄

if (Objects.isNull(price) || price.compareTo(BigDecimal.ZERO) < 0) {
throw new IllegalArgumentException();
}
public Menu changePrice(final UUID menuId, final MenuPriceChangeRequestDto request) {
Copy link

Choose a reason for hiding this comment

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

가격 변경 로직이 깔끔하게 정리되었네요 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants