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

[기능] order 도메인 구조 수정 #159

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

kimhyun5u
Copy link
Member

@kimhyun5u kimhyun5u commented Sep 19, 2024

💡 다음 이슈를 해결했어요.

Issue Link


💡 이슈를 처리하면서 추가된 코드가 있어요.

  • OrderFactory.java
    public class OrderFactory {
    private final SingleStoreOrderValidator singleStoreOrderValidator;
    private final StockRequester stockRequester;
    private final PriceChecker priceChecker;
    private final WithdrawPointService withdrawPointService;
    private final DateTimeProvider dateTimeProvider;
    public OrderFactory(SingleStoreOrderValidator singleStoreOrderValidator,
    StockRequester stockRequester,
    PriceChecker priceChecker,
    WithdrawPointService withdrawPointService, DateTimeProvider dateTimeProvider) {
    this.singleStoreOrderValidator = singleStoreOrderValidator;
    this.stockRequester = stockRequester;
    this.priceChecker = priceChecker;
    this.withdrawPointService = withdrawPointService;
    this.dateTimeProvider = dateTimeProvider;
    }
    public Order createOrder(Customer requester, List<CartItem> cartItems) {
    Store store = singleStoreOrderValidator.check(cartItems);
    List<CartItem> stockDecreaseSuccessCartItems = null;
    try {
    stockDecreaseSuccessCartItems = stockRequester.request(cartItems);
    List<OrderItem> orderItems = priceChecker.check(store, cartItems);
    withdrawPointService.withdraw(requester, orderItems);
    return new Order(requester, store, orderItems, dateTimeProvider.now());
    } catch (Exception e) {
    if (stockDecreaseSuccessCartItems != null) {
    stockRequester.rollback(stockDecreaseSuccessCartItems);
    }
    throw e;
    }
    }

💡 이런 고민을 했어요.

  • Order 는 도메인으로서의 역할만 하도록 변경하고 기존 생성 로직을 분리하고 싶었음
  • 의존성 주입을 OrderFactory 를 통해 진행하면서 테스트 작성이 조금 더 용이해짐

✅ 셀프 체크리스트

  • 내 코드를 스스로 검토했습니다.
  • 필요한 테스트를 추가했습니다.
  • 모든 테스트를 통과합니다.
  • 브랜치 전략에 맞는 브랜치에 PR을 올리고 있습니다.
  • 커밋 메세지를 컨벤션에 맞추었습니다.
  • wiki를 수정했습니다.

- Order 생성 시 필요한 검증 및 처리를 팩토리패턴으로 분리해 처리하도록 수정
@kimhyun5u kimhyun5u added the 💪 Improve 기능 고도화 & 개선 label Sep 19, 2024
@kimhyun5u kimhyun5u added this to the 리팩토링 milestone Sep 19, 2024
@kimhyun5u kimhyun5u self-assigned this Sep 19, 2024
@kimhyun5u kimhyun5u linked an issue Sep 19, 2024 that may be closed by this pull request
@kimhyun5u kimhyun5u marked this pull request as ready for review September 19, 2024 10:33
Copy link
Member

@Dr-KoKo Dr-KoKo left a comment

Choose a reason for hiding this comment

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

계속 미뤄두고 있었는데.. 처리해주셔서 감사합니다!

src/main/java/camp/woowak/lab/order/domain/Order.java Outdated Show resolved Hide resolved
this.dateTimeProvider = dateTimeProvider;
}

public Order createOrder(Customer requester, List<CartItem> cartItems) {
Copy link
Contributor

Choose a reason for hiding this comment

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

현재는 withdraw가 마지막 로직이라 상관은 없겠지만, withdraw이후에 exception이 발생하게 된다면 잔고의 롤백 처리는 안될거같아요! 혹시 모르니 추가해주는게 좋을거 같아요!

Copy link
Member Author

@kimhyun5u kimhyun5u Sep 22, 2024

Choose a reason for hiding this comment

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

@Hyeon-Uk 혹시 롤백이 발생하지 않는 경우의 예시를 들어주실 수 있나요?? 정확히 어떤 부분인지 모르겠어요

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💪 Improve 기능 고도화 & 개선
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[수정] Order 도메인 구조 수정
3 participants