[2기-유민환] SpringBoot Part2 Weekly Mission 제출입니다.

[yuminhwan]

안녕하세요 아만드 멘토님!!!

이번 미션에서는 피드백 적용과 최대한 MVC패턴에 맞게 구현하려고 해보았습니다. Part1에 비해 설계나 구현이 바뀐 것이 많아서 깔끔한 코드를 유지 못한 것 같습니다 😂 피드백 적용하면서 하나씩 고쳐 나가겠습니다!

이번 리뷰도 잘 부탁드립니다!! 😄

📌 과제 설명

• 바우처 관리 애플리케이션
  • 최대한 많은 단위 테스트코드를 작성하였습니다.
    • Hamcrest 대신 AssertJ를 사용하였습니다.
  • customer 테이블 정의 및 추가, JdbcTemplate을 사용해서 고객을 구현하였습니다.
  • voucher 테이블 정의 및 추가, JdbcTemplate을 사용해서 바우처를 구현하였습니다.
  • 바우처 지갑을 구현하였습니다.
    • 각 테이블은 resources폴더 아래 sql파일에 있습니다.
    • wallet 테이블은 따로 정의 하지 않고 voucher 필드에 customer id를 추가하였습니다.
  • 블랙 리스트의 경우 이전과 동일하게 파일로만 읽을 수 있도록 하였습니다.
    • 사실 고객 필드로 구분했어야 하는 데 구분하기 전에 많은 부분을 구현해버려서 다음 미션때 개선하도록 하겠습니다....
  • 이전 미션들의 클래스(MemoryVoucherRepository, FileVoucherRepository 등)에는 추가된 기능을 구현하지 않았습니다.

👩‍💻 요구 사항과 구현 내용

CREATE TABLE customer
(
    customer_id   BINARY(16) PRIMARY KEY,
    name          varchar(20) NOT NULL,
    email         varchar(50) NOT NULL,
    last_login_at datetime(6)          DEFAULT NULL,
    created_at    datetime(6) NOT NULL DEFAULT CURRENT_TIMESTAMP(6),
    CONSTRAINT unq_user_email UNIQUE (email)
);

CREATE TABLE voucher
(
    voucher_id  BINARY(16) PRIMARY KEY,
    customer_id BINARY(16),
    type        TINYINT UNSIGNED NOT NULL,
    discount    INT UNSIGNED     NOT NULL
);

• 테이블 관계상 voucher의 customer_id는 외래키로 가져야 되지만 테스트시 어려움이 있어 삭제하였습니다.
• 바우처 지갑 기능은 VoucherService쪽에서 처리하도록 하였습니다.
• name, email 같은 원시값을 포장하여 VO로 만들었습니다.
• 입력 같은 경우 DTO를 사용하여 구현하였습니다.
• Voucher의 경우 상속이 더 맞는 듯하여 추상클래스로 구현하였습니다.

✅ 피드백 반영사항

• checked exception을 모두 삭제하고 unchecked exception만 발생하도록 변경하였습니다.
  • unchecked exception을 통해 재입력을 받을 수 있게 처리하였습니다.
• VoucherType에서 Voucher를 생성하도록 하였습니다.
  • MenuType의 경우 바꾸지 않고 원래 구조대로 구현하였습니다.
• 로그를 콘솔에 출력하지 않고 error 레벨 이상일때만 파일로 관리하도록 하였습니다.
  • 또한, 의도치 않은 에러가 발생했을 때만 로그를 찍도록 하였습니다.
• @Value 를 통해 파일 경로를 가져오도록 하였습니다.
• 인스턴스 생성시 블랙 리스트를 미리 가져오도록 하였습니다.
• Mockito를 사용했습니다.

✅ PR 포인트 & 궁금한 점

1. 테이블 설계시 wallet 테이블을 새로 생성하는 것이 아닌 voucher테이블에서 customer에 대한 정보를 가지고 있는 것이 맞다고 생각하였습니다. 어떤 설계가 더 좋은 설계인가요?

2. 객체의 경우 되도록 불변을 유지하고 getter와 setter를 사용하지 않는 것이 좋다고 알고 있습니다. 하지만 이번 미션의 경우 getter와 setter 사용이 불가피 하고 그로 인해 불변을 유지하지 못하게 되었습니다. DB와 매핑되기 위해서 어쩔 수 없는 부분일까요?

3. 추가적으로 도메인 모델은 불변을 유지하는 것이 좋다고 알고 있는 데 이는 VO에 해당하고 Entity에는 해당되지 않는 말일까요?

4. 이번 미션을 하면서 VO를 사용해보았습니다. 하지만 customer에서 name을 원시값으로 꺼내기 위해 아래와 같이 구현하였는 데 이것보단 VO 객체를 주는 것이 더 맞는 방향일까요?

   public String getEmail() {
           return email.getEmail();
   }
   
   public Email getEmail() {
           return email;
   }

5. 도메인이 View에 노출되면 안 좋은 것으로 알고 있습니다. 그렇다면 View에서 VO로 입력받아 가져오는 것도 지양해야 할까요?

6. Repository 단위 테스트의 경우 검증하는 용도로 다른 메서드를 사용해도 되나요? 예를 들어 save를 테스트한다고 하면 findAll로 검증하는 식으로요!

7. 현재는 Service 구현체가 하나라서 인터페이스를 별도로 생성하지 않았는 데 인터페이스를 생성하는 것이 좋을까요?

8. 현재는 Service 단에서 CRUD 로직만 처리하고 있어 도메인 로직을 어떻게 사용하고 활용하는 지 감이 안오는 것 같습니다. 팀 미팅시간에 어떻게 도메인 로직이 이루어지고 저장되어지는 지 설명해주실 수 있을까요?

9. 저번 피드백에서 말씀하신 것처럼 Mockito를 사용해봤는 데 알맞게 사용한 지 확신이 안가네요 ㅎㅎ 고쳐야 할 부분이 있을까요?

오래한 만큼 질문이 많아진 것 같습니다. 😂

순수 자바로만 구현하다가 DB, Spring으로 넘어오니 깔끔하게 구현이 안되는 것 같네요.

피드백 받으면서 고쳐나가도록 하겠습니다.

나머지 미비한 점이나 고쳐야 할점 말씀해주시면 감사하겠습니다!!!

[bosuksh]

과제하느라 고생 많으셨어요~
오래 고민한만큼 잘 짜주셔서 생각보다 코멘트 달 부분이 많이 없었네요

1. Wallet이 Entity로써 가지는 역할이 생긴다면 새로운 테이블로 가지는게 좋겠죠? 그러나 지금은 단순히 VoucherList를 저장하는 용도라고 한다면 지금 당장은 Wallet의 테이블을 생성할 필요는 없을것 같습니다.
   그러나 User가 가지는 Wallet이 여러개가 생길수 있게 되고 이런식으로 확장을 생각해볼 순 있을것 같네요

2. DB와 매핑 되더라도 불변을 유지할 수 있다고 생각합니다. 애초에 모든 필드가 객체 생성일때만 값이 들어가도록 허용하게 만드는게 제일 좋겠죠? 그리고 예를 들면 어떤 비즈니스 로직에 의해서 바뀔 수 있는 필드, 예를 들면 assign�Customer 같은 건 허용하더라도 저런 식으로 set 대신 적절한 이름을 두는게 좋은것 같아요

3. 아니요 Entity도 불변으로 유지하는게 좋습니다.

4. VO 내부에 각 필드로써 사용할 만한 값이 있다면 객체로 반환하는게 좋겠죠. 예를 들면 Name 객체에서 FirstName, LastName으로 구분짓는다면요.. 그런데 만약 단순히 필드 하나에 원시 타입이라면 그 원시타입으로 불러와도 될것 같네요

5. 저는 개인적으로 VO가 도메인에 사용된다면 그 VO도 최대한 노출을 지양하는 편입니다. 그러나 중복이 많이 생긴다면 VO정도는 공유해도 괜찮을것 같아요

6. 넵넵 안그러면 검증할 방법이 쉽지 않을것 같은데요

7. 구현체가 하나로 확정적이면 굳이 Interface를 생성할 필요는 없을 것 같습니다.

8. 어떻게 보면 Validation도 도메인 로직인셈이죠 또 예를 들면, Voucher 클래스에 discount method를 두고 그 VoucherService에서 discount를 할수도 있겠지만, 도메인 내부에서 계산을 하는식으로 하는 방법도 도메인 로직을 이용하는 방법일 수 있겠네요

9. Mockito 잘 사용해 주셨는데 테스트 코드에 @nested가 너무 많아서 오히려 가독성이 떨어지는 면도 있는 것 같아요. 단순히 메소드 하나를 테스트 할거면 굳이 Nested를 안붙이고 테스트 메소드 하나로 해결하는 방법이 더 간단할 수도 있습니다.

[bosuksh]

굳이 중복으로 생성한 이유가 있을까요?

[yuminhwan]

리팩터링을 하면서 나온 실수인 것 같습니다.
삭제하도록 하겠습니다!!

[bosuksh]

password는 노출안되도록 주의해주세요

[yuminhwan]

노출안되도록 습관화하는 자세를 가져야 겠습니다.
실무에서는 어떻게 설정파일을 관리하는 편인가요?? 🤔

[bosuksh]

lastLoginAt 보다는 updatedAt같은 표현이 좋아보입니다.
login의 개념이 여기에는 잘 없는듯해서

[bosuksh]

이렇게 Enum Type에 클래스를 넣어서 생성하게 해도 좋네요

[yuminhwan]

Enum은 정말 무궁무진한 것 같습니다!!

[bosuksh]

Suggested change

	@DisplayName("정해진 할인퍼센트로 할인한다.")
	@DisplayName("정해진 할인퍼센트로 할인한다.")

[bosuksh]

Suggested change

	@DisplayName("command를 통해 해당하는 VoucherType을 반환한다.")
	@DisplayName("command를 통해 해당하는 VoucherType을 반환한다.")

[bosuksh]

테스트 코드에 @nested가 너무 많아서 오히려 관리하기 힘들어지는 것 같아요
테스트 대상이 하나면 하나의 메소드안에서 해결하는것도 좋은것 같아요

[yuminhwan]

예외 사항이 많아서 BDD 방식을 연습해보았는 데 너무 과했군요 😅