refactor(order) : 주문 도메인 필드에서 타 도메인 제거 #208
Conversation
| // 연관 관계로 만들면..? 가격정보 도메인 내부로 들일 수 있음 | ||
| @JoinColumn(name = "option_id", updatable = false) | ||
| @ManyToOne(fetch = FetchType.LAZY) | ||
| private Option option; | ||
| private Long optionId; | ||
|
|
| public boolean checkCanWithDraw() { | ||
| return this.equals(OrderStatus.CONFIRM) || this.equals(OrderStatus.APPROVED); | ||
| } |
There was a problem hiding this comment.
boolean 타입 리턴이면 check 안붙이고 canWithdraw 해도 될 거 같아용
check 는 약간 validation 하는 느낌...? 같은데 어떻게 생각하시는지
There was a problem hiding this comment.
음 다시보니깐 상태를 그대로 반환하는느낌이있어서
checkMethodIsPayment
이런걸 canMethodIsPayment 로가는 것보단...
그냥 is가 나을것같아유
isStatusWithDraw
isMethodPayment
요래 수정했슴다!
There was a problem hiding this comment.
getter에서도 boolean 타입은 get컬럼명이 아니라 is컬럼명으로 선언해서 불러오더라구요. is로 지정하는 거 좋은 것 같습니다!
There was a problem hiding this comment.
@cofls6581 맞아유 불리언 리턴은 is 통일이 맞는듯!
| */ | ||
| @Component | ||
| @RequiredArgsConstructor | ||
| public class OrderValidator { |
There was a problem hiding this comment.
validator 로 아예 분리시켰네요 이렇게 나누면 host 도 나눠야할 필요성이 있을듯,,.?
There was a problem hiding this comment.
이게 밸리데이터로 분리시킨이유가
이벤트를 레포지토리로 조회할 필요성이 있어서 그래요!
그래도 단위테스트가 더 편리해지는 이점이있으니
검증로직 밸리데이터로 빼는거 좋긴해유
|
Kudos, SonarCloud Quality Gate passed!
|
| */ | ||
| @Validator | ||
| @RequiredArgsConstructor |
There was a problem hiding this comment.
@gengminy 밸리데이터 커스텀 어노테이션 추가했습니당~!
|
|
||
| /** 환불 할 수 있는 주문인지 체크합니다. */ | ||
| public Boolean isRefundDateNotPassed(Order order) { | ||
| List<Event> events = eventAdaptor.findAllByIds(getEventIds(order)); |
There was a problem hiding this comment.
validator 활용해서 주문 응집도 높인 거 굳!
| } | ||
|
|
||
| /** 주문 방식이 결제 방식인지 검증합니다. */ | ||
| public void validMethodIsPaymentOrder(Order order) { |
There was a problem hiding this comment.
영상듣다보면 도메인 내에서 이루어지는 간단한 검증 정도는 도메인 내에서 처리해도 괜찮으니 validator로 넘길지말지 적절하게 판단하면 된다는 언급이 있더라구요.
validator에서 validation을 다 처리하면 한 눈에 모아보며 관리할 수 있고 validaiton이 빠진 주요 로직만 도메인 서비스 코드에서 볼 수 있는 장점이 있는 것 같은데 한편으로는 간단하게 처리할 수 있는 부분이 되려 복잡해지는 것 같기도해요.
어떻게 생각하시나유
There was a problem hiding this comment.
@cofls6581
다른 도메인이 다 이렇게 처리할 필요가 없습니다
다만 저는 이렇게 할수밖에 없는 구조라서 뺀거에유
객체 연관관계를 없애니깐
더이상 lazy loading 으로 도메인 엔티티 내부에서 검증을 수행할 수가없는 경우에요
가령 이벤트 의 환불정보를 원래는 엔티티그래프로 불러와서 수행하면 되는건데
지금 상태는 event 의 id값만 저장하다보니
따로 validator를 빼서 event repository를 주입받아서 사용하고있어요
그리고 넘기는김에 다 넘겨서 처리했어요!
밸리데이션을 그냥 밸리데이션 한쪽에 모으는게 테스팅도 편하니깐
There was a problem hiding this comment.
아 도메인 내에서 이루어지는 간단한 검증이 다른 도메인 받아서 사용하는 거 말구 Order만 사용하는 검증 얘기였어요! 하하
테스팅 위해서 넘기는 김에 다 넘겨서 처리하신거군요!
의견 공유 감사합니당ㅎㅎㅎ
개요
작업사항
주문 도메인에서 타 도메인 필드를 아예 제거했습니다.
주문 도메인은 ddd의 aggregate root 라고 부를만한 상태가 된것같습니다?
타 도메인을 없앰에 따라서 검증로직을 이전하기위해 orderValidator 를 추가했습니다.
작업하면서 주문성공시 티켓 발급과 같은 타도메인과 관련된 로직은 이미 도메인 이벤트로
빼두면서 작성했기때문에 타도메인을 Lazy loading 하는 검증 로직을 옮기는게 관건이였습니다.
orderValidator 관련 내용은
https://www.youtube.com/watch?v=dJ5C4qRqAgA&t=4691s
위영상 참고하면 좋을 것 같습니다. 내용좋고 저도 한 5번은 본듯
다들 한번씩 보세유 도메인 이벤트 관련 내용도 나와요 꼭 보세요
밸리데이션 로직이 타클래스로 빠짐에 따라서 Order 도메인 커맨드 메소드 테스트가 수월해졌습니다.
결론은 이게 더 나은 구조인것 같긴합니다.
밸리데이션 관련 테스트도 수월해지고, 테스트 코드도 한결 더 가벼워진느낌입니다.
변경로직
주문에서 -> 티켓으로 변환하는 로직에 옵션을 찾아서 넣어줘야했는데
이때 OptionAdator가 필요했었습니다.
관련 생성자 소스가 도메인 내부에 포함되어있어
도메인 서비스로 노출시키기 위해서 어쩔수 없이 리팩한번 했습니다. 사실 맵퍼가 더적절한것 같습니다.
@sanbonai06 flatmap 사용했으니 참고 바라요!