-
Notifications
You must be signed in to change notification settings - Fork 2
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
[#4] 상품 등록, 조회, 수정, 삭제 #9
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@requiredargsconstructor 를 사용한 이유
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
인터페이스 사용 이유
|
||
import java.util.List; | ||
|
||
public interface ItemService { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
인터페이스 사용 하는 나의 생각이 있으면 좋을 것 같아요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
말씀 드렸지만 클래스 네이밍을 좀 더 구체적으로 명명해 주시면 좋을 것 같습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@f-lab-thor 인터페이스를 사용하는 이유는 기존 구현 객체와 비즈니스 로직이 다른 기능을 추가해야할 경우 다른 구현 객체를 만들어서사용하면 되기 때문에 유지보수 측면에서 좋기때문입니다.
인터페이스로 테스트 하는 것이 효율적인 이유도 있습니다. 인터페이스로 단위 테스트를 작성하면 테스트 코드에서 구현체만 바꿔가며 테스트할 수 있기 때문에 효울적입니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@f-lab-thor 클래스 네이밍을 구체적으로 한다는게 ItemOneService, ItemListService, ItemUpdateService 처럼 목적에 맞게 분리한다는 뜻이 맞을까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
어떤 역활을 하는 클래스인지 좀 더 명확하게 네이밍하여 클래스가 비대해 지는것을 막고 역활에 맞는 로직만 수행하기 위함인데요
해당 인터페이스를 정의 하실 때, 어떤 역활을 위한 클래스를 설계 하신지 생각해보면 좋을 것 같아요
스펙을 보니 단순 CRD를 위한 클래스 같은데 관련 내용으로 명명하게 된다면 해당 역활만 하는 클래스가 되지 않을까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CQRS ( Command and Query Responsibility Segregation )
명령을 처리하는 책임과 조회를 처리하는 책임을 분리시키는 것
https://learn.microsoft.com/ko-kr/azure/architecture/patterns/cqrs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CQRS까지 필요가 있을까요??
제가 질문 드린 의도는 클래스 설계 할 때 역활을 명확하게 구분 짓고
그에 상응하는 네이밍을 해야 슈퍼 클래스가 생기지 않고 유지보수와 코드수정 용이하다는 장점이 있다 라는 것을 말씀 드리고 싶어서 였습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@f-lab-thor service 책임 분리를 하는 방식이 operation의 타입에 따라나누는 것과 Command, Query로 나누는 것 2가지가 있는 것 같아서 고민하다가 후자의 방법으로 수정할까 고민중이었습니다 ~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zzangoobrother 리팩토링 전에 맞춰보면 좋을 것 같아요 ~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
커맨드와 쿼리를 왜 나누자는 말이 나왔을까?? 에 대한 의문을 가지시고 그 이유를 이해해 보시면 좋을것 같아요
단순히 이런게 있다고 하니 이번 프로젝트에 적용해 보고싶으신거라면 좋습니다.
하지만 실무에 적용한다고 하면 굳이? 라는 생각이 드네요
물론 요구사항 분석과 충분히 미래에 대해 예측 가능한 내용으로 뒷받침 되어 그렇다면 납득은 될 수 있을거 같습니다.
대부분 근데 그럴거라고 생각했던 것들이 그렇지가 않고 일단 적용하고 필요해 졌을때, 리팩이나 구조를 바꾸는게 낫더라고요
YAGNI 라는 법칙이 있는데 한번 보시면 좋을것 같습니다.
그리고 그걸 떠나서 현재 인터페이스를 보면 커맨드 쿼리를 분리 되어 있지도 않고 클래스 네이밍도 커맨드, 쿼리 구분이 되지 않아서 네이밍도 바꿔 주셔야 해요
} | ||
|
||
@Override | ||
@Transactional(readOnly = true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
마찬가지로 사용해야 하는 이유
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@f-lab-thor @Transational 어노테이션의 readOnly 옵션을 true로 사용하는 이유는 3가지 입니다.
첫번째로는 조회한 데이터를 return 한다고 해도 의도치 않게 데이터가 변경되는 일을 사전에 방지해주기 때문입니다.
두번째로는 해당 옵션인 경우 스냅샷저장, dirty check의 작업을 행하지 않아 성능이 향상됩니다.
dirty checking은 상태 변경 검사이고 dirty는 상태의 변화가 생긴 정도입니다.
세번째 이유는 상황에 따라 DB 서버의 부하를 줄이고 약간의 최적화를 할 수있기때문입니다.
MySQL을 사용할 때 데이터가 날아가는 것을 방지하기 위해서 이중화 구성을 하는 경우가 있는데 DB가 master와 slave로 나누어져 있다면 readOnly 옵션이 true로 있는 경우에는 읽기 전용으로 master가 아닌 slave를 호출하게 됩니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
네 반대로 데이터가 변경되는것을 알아야 할 때도 있죠? Replica Lag 으로 인한 데이터 유실이 있어서는 안되는 케이스라면 반드시 Master에서 조회가 필요 할 수 도 있다는 점은 참고해 주세요~
} | ||
|
||
@Override | ||
public List<Item> getItems() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기서 find 라고 하면 어떤 늬앙스가 될까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@f-lab-thor getXXX 은 해당 파라미터로 Entity를 리턴하고 조회 결과가 없다면 Exception을 발생시킵니다. findByXXX는 Optional를 리턴합니다.
@@ -0,0 +1,21 @@ | |||
server: | |||
shutdown: graceful |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
어떤 의미인지 알고 계시면 좋을 것 같아요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@f-lab-thor graceful 옵션은 서버 종료시 새로운 요청에 대해서는 수신하지 않고 기존 요청에 대해서는 완전히 처리를 진행하고 서버종료를 마무리합니다. 신규 요청을 받지 않은 채로 기존 요청은 마무리함으로써 욫ㅇ에 대한 응답의 유실은 최소한 피할 수 있게 됩니다.
@ActiveProfiles("test") | ||
@SpringBootTest | ||
@Transactional | ||
@DirtiesContext(classMode = DirtiesContext.ClassMode.BEFORE_CLASS) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
스프링 컨텍스트는 기본적으로 캐쉬 되어 최대한 활용하는 전략이
나중에 테스트 코드 수행 할 때 빨라서 유지보수 하는데 도움이 됩니다
정말 필요한 건지 다시 한번 확인해 주세요~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@f-lab-thor 테스트 격리 목적으로 추후에 사용될 것 같아 미리 추가해두었는데 사용이 지양되는 어노테이션이니 제거했습니다 ~
|
||
@Builder | ||
public Item(String itemName, Long itemPrice, Long itemStock) { | ||
if (StringUtils.isBlank(itemName)) throw new InvalidParameterException("Item.itemName"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
return itemRepository.save(item); | ||
} | ||
|
||
private void validCheck(Item item) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이런 단순 유효 체크는 앞단(controller) 에서 이미 한거 같은데 또 할필요 있을까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@f-lab-thor controller에서 itemToken이 생성되기 전에 유효성 체크를 하기 때문에 생성이 되었는지 확인할 수가 없어서 service단에서 한번더 확인했습니다.
@f-lab-thor spring framework에서는 객체가 의존하는 또 다른 객체를 외부에서 선언하고 이를 주입받아 사용해야 합니다. 의존관계를 주입하는 방법은 아래와 같이 3가지가 있습니다.
이 중 생성자 주입 방식을 사용하는 것이 권장되는데 그 이유는 여러가지가 있지만 그 중 크게 2가지입니다. 하나, 객체의 생성자는 객체 생성시 1회만 호출된다는게 보장되기 때문에 객체의 불변성을 확보할 수있습니다. 둘, 생성자 주입 방식을 사용하면 순환 참조 에러방지를 할 수 있습니다. 순환 참조 문제를 해결하는 것이 아니라, 순환 참조 문제를 애플리케이션 실행 시점에 알려줘서, 실제 서비스되기 전에 개발자로 하여금 순환 참조 문제를 해결할 수 있게 해 줍니다. @requiredargsconstructor는 final 필드에 대해 생성자를 만들어주는 lombok의 annotation이며 Constructor Injection을 임의의 코드 없이 자동으로 설정할 수 있어 새로운 필드를 추가할 때 다시 생성자를 만드는 번거로움을 없앨 수 있습니다. |
@f-lab-thor 도메인의 조회화 저장을 담당하는 interface를 각각 선언했고 xxxReader와 xxxStore의 구현체를 만들고 이를 infrastructure layer에 두었습니다. DDD에서 infrastructure layer의 역할은 상위 계층을 지원하는 일반화된 기술적 기능을 제공하는 것입니다. 이를 의존성 역전 원칙 개념을 활용해서 인터페이스를 두고 그 구현체를 만들어 사용하였습니다. xxxReaderImpl의 실제 구현은 Spring JPA를 활용하고 있지만 추후에 Spring JDBD 또는 MyBatis와 같이 다른 low level 구현을 사용하더라도 xxxService에서는 특별한 변경 없이 사용이 용이하다는 장점이 있습니다. |
|
||
import java.util.List; | ||
|
||
public interface ItemService { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
어떤 역활을 하는 클래스인지 좀 더 명확하게 네이밍하여 클래스가 비대해 지는것을 막고 역활에 맞는 로직만 수행하기 위함인데요
해당 인터페이스를 정의 하실 때, 어떤 역활을 위한 클래스를 설계 하신지 생각해보면 좋을 것 같아요
스펙을 보니 단순 CRD를 위한 클래스 같은데 관련 내용으로 명명하게 된다면 해당 역활만 하는 클래스가 되지 않을까요?
} | ||
|
||
@Override | ||
@Transactional(readOnly = true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
네 반대로 데이터가 변경되는것을 알아야 할 때도 있죠? Replica Lag 으로 인한 데이터 유실이 있어서는 안되는 케이스라면 반드시 Master에서 조회가 필요 할 수 도 있다는 점은 참고해 주세요~
public interface ItemReader { | ||
Item getItemBy(String itemToken); | ||
|
||
List<Item> getItems(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Item 엔티티는 JPA 스펙에 종속된 클래스인데,
해당 인터페이스에 두는건 안 맞지 않을까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@f-lab-thor JPA 스펙에 종속된 엔티티 클래스의 사용범위는 어디까지 일까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
제가 이해한 의미를 조심스럽게 말씀 드리면,
ItemReader 인터페이스가 Item entity 클래스를 알아야 하나? 이런 말씀인거 같습니다.
인터페이스를 사용하는 클라이언트에게 entity 클래스 보다는 vo 객체를 만들어 전달하는게 좋지 않을까?
라는 의미이지 않을까 생각됩니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zzangoobrother ItemReader의 구현체에서 itemRepository에 접근해서 item엔티티를 리턴 받도록 구현되어 있는 상태입니다. vo 객체로 어떻게 변환이 가능할까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
itemVO 라고 클래스 만드셔서 dto로 변환 하듯이 하셔도 될 겁니다.
제가 이해한거는 jpa에서 한번 꺼내고 리턴하는데 entity를 리턴하느거 보다는
중간 변환 객체를 사용해서 리턴하는데 entity를 보호하는게 좋다라고 이해했습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
우선 이미 은지님이 그 답을 여기에 말씀해 주셨네요
해당 인터페이스가 존재하는 이유는 ItemReader의 구체적인 방식을 의존하지 않고
추상화된 또는 고수준의 역활만 바라보게 하여
변경이 있어도 그것을 사용하는 쪽에 영향이 없게 하기 위함입니다.
내가 jpa를 더이상 안쓰고 다른 구현체를 사용한다고 했을때, 도메인 영역까지 영향이 침범 되지 않을까요?
entity 패키지 명만 보시면 아실거에요
인터페이스가 도메인 영역에 있고, 인터페이스 구현체가 infrastructure에 있다는 것은 DIP 원칙이 훤히~~ 보이는 구조로 말씀드렸습니다.
@Setter | ||
@ToString | ||
public static class UpdateItemRequest { | ||
// private String itemToken; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
안쓰는건 제거
작업 내용
issue: #4
참고 사항