🚀 1단계 - 리팩터링(상품) by shinseongsu · Pull Request #241 · next-step/ddd-tactical-design

shinseongsu · 2023-09-24T18:43:12Z

늦게 제출해서 죄송합니다

요구사항대로, tobe 패키지를 만들어 추가되는 domain은 그곳에 추가하였습니다.

kyucumber

안녕하세요

몇가지 코멘트 남겨두었으니 확인 부탁드립니다.

kyucumber · 2023-09-25T14:06:01Z

+        this.purgomalumClient = purgomalumClient;
+    }
+
+    @Transactional


중요한 부분은 아니나, 빠트리지 않도록 클래스 레벨로 어노테이션을 달아도 괜찮을 것 같네요.

그렇군요 @transactional 이 중복으로 메소드에 붙어 있네요

kyucumber · 2023-09-25T14:06:51Z

+@Service
+public class ProductService {
+    private final ProductRepository productRepository;
+    private final ProductMenuService productMenuService;


인터페이스를 통해 Menu 컨텍스트와 결합을 낮춰주신 부분이 좋네요 👍

kyucumber · 2023-09-25T14:08:05Z

+    }
+
+    @Override
+    public void validMenuDisplay(final UUID productId) {


메서드 이름은 valid이지만 실제로 validate의 목적이 아니라 가격을 확인하고 조건에 맞지 않으면 menu의 노출 여부(displayed)값을 변경하고 있습니다. 메서드 이름을 변경하는게 어떨까요?

changeMenuDisplay 같은 이름으로 수정하도록 하겠습니다.

kyucumber · 2023-09-25T14:08:33Z

+    }
+
+    @Override
+    public void validMenuDisplay(final UUID productId) {


인터페이스를 통해 결합도를 나누어 주신 부분은 좋으나 지금의 구현이라면 Spring의 ApplicationEvent를 사용해도 좋을 것 같다는 생각이 드네요.

마지막 강의때 @eventlistener 강의 해주었는데 써보면 좋겠군요 ㅎㅎ

kyucumber · 2023-09-25T14:09:19Z

+    @Embedded
+    private ProductName name;
+
+    @Embedded
+    private ProductPrice price;


ValueObject 사용 👍

kyucumber · 2023-09-25T14:10:13Z

+        final Product product = productRepository.findById(productId)
+            .orElseThrow(NoSuchElementException::new);


이번 미션은 도메인 쪽만 리팩터링하는거긴 하지만 참고로 코멘트 남깁니다.

NoSuchElementException이 발생하면 어떤 에러 응답이 나가게 될까요? 이와 관련해서 RestControllerAdvice 등으로 처리해줄 필요는 없을까요?

GlobalRestController 쓰면 더 정확히 응답값을 받아 볼 수 있겠군요 ㅎㅎ

kyucumber · 2023-09-25T14:12:34Z

+import java.math.BigDecimal;
+import java.util.UUID;
+
+public class ProductInfo {


지금 아래와 같이 구성해주신 것 같네요

Product 관련 Request, Response

ProductInto

Product(Entity)

이렇게 되면 ProductInfo라는 도메인 클래스에 도메인 로직(Value Object 등)이 적용되어야 하지 않을까요? 데이터베이스 레이어와 결합되고 있는 Entity가 도메인 로직을 가지는 것 보단 도메인 클래스가 도메인 로직을 가지는게 적절해보여요.

DM으로도 드렸지만, Productinfo가 DTO 역활을 하고 싶어서 만든것이라서, @kyucumber 리뷰어님 말씀대로 entity 보다는 vo를 만들어 사용하는것이 DB에 영향에 덜 갈것같습니다 ㅎㅎ

shinseongsu added 3 commits September 25, 2023 03:04

refactor: dto 추가 및 name, price 도메인 추가

c72779d

fix: 객체로 묶여져 있는거 한번더 getter 사용

b1b331e

test: domain 및 service 테스트 코드 추가 및 수정

93e8cda

kyucumber requested changes Sep 25, 2023

View reviewed changes

shinseongsu added 5 commits September 26, 2023 23:17

fix: Transactional class 범위로 지정

acf01bd

fix: 조건에 맞으면 display 변경되도록 하는 메소드 이름 수정

c7a97c3

fix: Event 성으로 다른 도메인 Service로 데이터 보내기

1d361ee

feat: Global 예외 처리 컨트롤러 추가

3618040

fix: vo 에서 validator 체크하도록 수정

953cbd4

		final Product product = productRepository.findById(productId)
		.orElseThrow(NoSuchElementException::new);

Conversation

shinseongsu commented Sep 24, 2023

Uh oh!

kyucumber left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants