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

OrderService주문 사전 저장의 일부 로직에 대한 책임을 OrderService -> orderForm 도메인 모델로 책임 변경 리팩토링 #167

Conversation

daadaadaah
Copy link
Collaborator

@daadaadaah daadaadaah commented Aug 6, 2023

Why

  • 주문 사정 저장에 대한 OrderService의 책임이 너무 많다보니, OrderService의 placeOrderInAdvance애 사용되는 private 함수가 많아져서, 다음과 같은 불편함을 해결하고자, OrderForm이라는 도메인 모델을 만들어서 리팩토링 했습니다. private -> public 으로 바꿀까? 했는데, 그러면 캡슐화 관점에서 너무 많은 함수들이 노출되어서 좋지 않는 방법이라고 생각하여, 도메인 모델 하나 만들어서 거기서 관리하자 라는 생각을 하였습니다.

불편함 1. private 함수 너무 많아서, 가독성이 좋지 않았습니다.

불편함 2. private 함수이다보니, 테스트 코드 작성하는 것이 어려웠습니다.

  • 비즈니스 로직테스트 코드로 문서화 하고 싶었는데, 비즈니스 로직이 대부분 private 함수 내에 있어서, 테스트 코드 작성하는데 어려움이 있었습니다. 공부해본 결과, 리플렉션으로 private 함수를 테스트 할 수 있다고 하지만, 리플랙션 사용은 지양하는 것이 좋고, 만약 private를 테스트 하고 싶으면, 그건 클래스로 분리하라는 신호라는 글을 보고, 도메인 모델 클래스 만들어서 해결하면 되겠다라는 생각을 했습니다.

불편함 3. private 함수이다보니, 로그 AOP 적용이 어려웠습니다.

  • 로그 추적 AOP 만들어서 함수 호출 흐름 파악하고 싶었는데, private 함수라 적용이 안되었습니다. 물론 리플랙션을 사용해서 private에도 AOP를 적용시킬 수 있지만, 리플랙션 사용은 지양하는 것이 좋아서, 리플랙션을 사용하지 않았습니다.

What

  • OrderForm 이라는 도메인 모델을 만들어서 OrderService의 placeOrderInAdvance의 일부 로직에 대한 책임을 부여하고, OrderForm의 단위테스트를 작성하여 비즈니스 로직을 문서화 할 수 있도록 함.
  1. 객체의 역할을 좀더 명확히 하기 위해 OrderForm -> OrderFormDto로 이름 변경
  2. OrderService에서 OrderFormDto 대신 OrderForm 사용하기
  3. validateHasDealProductUuid 책임을 OrderService -> OrderForm 으로 수정
  4. validateOrderQuantityInMaxOrderQuantityPerOrder 책임을 OrderService -> OrderForm
  5. preValidateOrderQuantityInInventory 책임을 OrderService -> OrderForm 으로 수정
  6. determineRealOrderQuantity 책임을 OrderService -> OrderForm 으로 수정

@daadaadaah daadaadaah force-pushed the refactor/placeOrderInAdvance-in-OrderService branch from 052ea54 to d32ce88 Compare August 6, 2023 17:03
@daadaadaah daadaadaah force-pushed the refactor/placeOrderInAdvance-in-OrderService branch from d32ce88 to 8c546af Compare August 6, 2023 17:03
@daadaadaah daadaadaah requested a review from f-lab-TJ August 9, 2023 08:47
@daadaadaah daadaadaah merged commit 55e716a into f-lab-edu:develop Aug 9, 2023
@daadaadaah daadaadaah changed the title OrderServiceplaceOrderInAdvance 리팩토링 OrderService주문 사전 저장의 일부 로직을 OrderService -> orderForm 도메인 모델로 책임 전가 리팩토링 Aug 9, 2023
@daadaadaah daadaadaah changed the title OrderService주문 사전 저장의 일부 로직을 OrderService -> orderForm 도메인 모델로 책임 전가 리팩토링 OrderService주문 사전 저장의 일부 로직을 OrderService -> orderForm 도메인 모델로 책임 전가 리팩토링 Aug 9, 2023
Copy link

@f-lab-TJ f-lab-TJ left a comment

Choose a reason for hiding this comment

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

👍

@daadaadaah daadaadaah changed the title OrderService주문 사전 저장의 일부 로직을 OrderService -> orderForm 도메인 모델로 책임 전가 리팩토링 OrderService주문 사전 저장의 일부 로직을 OrderService -> orderForm 도메인 모델로 책임 변경 리팩토링 Aug 10, 2023
@daadaadaah daadaadaah changed the title OrderService주문 사전 저장의 일부 로직을 OrderService -> orderForm 도메인 모델로 책임 변경 리팩토링 OrderService주문 사전 저장의 일부 로직에 대한 책임을 OrderService -> orderForm 도메인 모델로 책임 변경 리팩토링 Aug 10, 2023
@daadaadaah daadaadaah mentioned this pull request Aug 3, 2023
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