-
Notifications
You must be signed in to change notification settings - Fork 0
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
[#1] repository에서 도메인 모델 분리 #5
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.
좋은 시도 입니다. 한 가지 부분을 짚어보고 가시죠 ㅎ
List<BorrowEntity> save(List<BorrowEntity> borrowEntityList); | ||
Borrow findById(long borrowId); | ||
Borrow save(Borrow borrow); | ||
List<Borrow> save(List<Borrow> borrowList); |
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.
파라미터 타입과 리턴 타입에 List 를 쓰는게 놓을까요?
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.
대출이 이뤄질 때 회원과 도서가 1:N 이라고 생각해 Member
의 대출 로직에서 List를 리턴하게 했었습니다. 그 다음 대출 정보를 각각 하나씩 저장하는 것보다 List로 한 번에 저장하는 것이 성능면으로 더 좋다고 생각해서 파라미터로 List를 받게 했었습니다.
단점
- 회원과 도서가 1:1인 대출 정보를 저장할 때 List로 변환하여 전달해야 함.
- 각각의 대출정보에 대한 디버깅이나 예외처리가 어려울 수 있을 것 같음.
해결방법
1:N, 1:1 인 대출정보를 모두 받을 수 있는 메서드를 선언한다.
List<Borrow> save(List<Borrow> borrowList);
Borrow save(Borrow borrow);
-> 회원이 대출받을 수 있는 도서의 수는 제한되어있기 때문에 List로 받아서 저장해도 성능면에서 별 차이가 없을 것 같아 좋은 방법은 아닌 것 같습니다.
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.
오호 그렇군요 혹시 List 대신 Collection 이나 Iterable 을 쓰는 건 어때요?
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.
앗 그 생각은 못 해봤네요... List 대신 Collection을 사용함으로써 결합도를 낮출 수 있어서 더 좋은 선택인 것 같습니다.
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.
넵. 파라미터는 Collection 타입으로 받고, 리턴타입도 Collection 타입으로 리턴해서 호출한 클라이언트 쪽에서 원하는 Collection의 하위 타입으로 변환할 수 있게 하는 것이 좋을 것 같습니다.
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.
리턴 타입을 Collection 으로 하면 받는 곳에서 원하는 하위 타입으로 쓸 수 있을까요? 한 번 해보면 재밌을 것 같아요
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.
호출한 쪽에서 Collection으로 받고 Stream을 이용해서 List나 Set 또는 Array로 변환해서 사용하면 조금 번거롭더라도 특정 구현체에 대해서 종속되지 않으니 괜찮다고 생각했습니다..!
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.
in out 에 대해 오늘 시간에 다뤄볼 수 있겠네요! ㅋㅋ
방금 커밋은 PR을 따로 작성하려고 했는데 실수입니다 ㅠ |
} | ||
|
||
@Test | ||
@DisplayName("Mapper 테스트 : BorrowDto -> Borrow") | ||
public void checkMapperBorrowDtoToBorrow() { | ||
BorrowDto borrowDto = BorrowDto.builder() | ||
.member(MemberDto.builder().id(1).build()) | ||
.book(BookDto.builder().id(1).ISBN(9788936433598L).build()).build(); | ||
.book(BookDto.builder().id(1).ISBN(9788936433598L).build()) | ||
.period(new Period(LocalDateTime.now())) |
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.
혹시 Factory 패턴을 아시나요?
|
||
static class BorrowFactory implements BorrowObjectFactory { | ||
@Override | ||
public Borrow createBorrow() { |
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.
뭔가 일이 커졌군요 ㅋㅋㅋㅋㅋㅋ 저는 Period 를 두고 한 얘기 였어요 ㅎ
자세한건 내일 얘기로 풀어보시죠
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.
ㅋㅋㅋㅋㅋㅋ앗 팩토리 패턴에 대해서 공부해보고 적용할만한 곳이 어디일까 하다가 그만..
팩토리 패턴
Borrow, BorrowDto, BorrowEntity 들을 한 객체군으로 묶기 위해서 추상 팩토리 패턴을 적용해봤습니다. |
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.
승인합니다
repository에서 수행하던 도메인과 엔티티 간의 변환을 service에서 수행하도록 도메인 모델을 분리하여 도메인 모델을 보호함