-
Notifications
You must be signed in to change notification settings - Fork 247
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
수강신청 Step2 - 수강신청(도메인 모델) #393
base: parkje0927
Are you sure you want to change the base?
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.
2단계 미션 구현하느라 수고했어요. 👍
2단계임에도 불구하고 벌써 db까지 고려해 구현했네요.
그렇다보니 기존 구현 방식에 얽매이는 느낌입니다.
2단계는 db는 고려하지 않고 도메인 객체 설계와 구현에만 집중해 보는 것은 어떨까요?
this(0L, title, creatorId, 1, null, LocalDateTime.now(), null); | ||
} | ||
|
||
public Course(Long id, String title, Long creatorId, int cardinalNumber, Sessions sessions, |
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.
주생성자를 생성자에 마지막에 구현하는 것이 관례
public List<Session> getPossibleSessionList() { | ||
return this.sessions.getPossibleSessionList(); | ||
} | ||
|
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.
새로운 Session을 추가하는 addSession(Session session)과 같은 메서드도 추가하는 것은 어떨까?
private int size; | ||
|
||
private int width; | ||
|
||
private int height; |
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.
원시값 포장, 관련 있는 필드 값을 객체로 분리하는 차원에서 ImageFileSize, ImageSize와 같은 객체를 분리하는 것은 의미가 있을까?
|
||
ImageType() { | ||
|
||
} |
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.
이미지 파일이 유효한 파일인지에 대한 체크 로직을 이 객체에 구현하는 것은 어떨까?
public class FreeSessionType implements SessionType { | ||
|
||
private Long amount; | ||
private int maxCapacity; |
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.
amount, maxCapacity 인스턴스 변수로 초기화하고 아무 곳에서도 사용하지 않고 있다.
굳이 인스턴스로 구현할 필요가 있을까?
private LocalDate startDt; | ||
|
||
private LocalDate endDt; |
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.
이 두 개의 값은 서로 연관되어 있는 값이다.
이 두 개의 값을 가지는 객체를 분리하는 것은 어떨까?
분리한 후 시작일은 종료일보다 빨라야 한다와 같은 유효성 체크 로직을 추가하는 것은 어떨까?
private LocalDate startDt; | ||
|
||
private LocalDate endDt; | ||
|
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.
Session별로 강의 커버 이미지 정보도 가지고 있어야 하지 않을까?
sessionA = new Session(1L, "test1", SessionStatus.RECRUITING, new FreeSessionType(), LocalDate.now(), LocalDate.now()); | ||
sessionB = new Session(2L, "test2", SessionStatus.TERMINATION, new FreeSessionType(), LocalDate.now(), LocalDate.now()); | ||
sessionC = new Session(3L, "test3", SessionStatus.RECRUITING, new FreeSessionType(), LocalDate.now(), LocalDate.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.
Session 객체와 같이 인스턴스 변수가 많은 객체를 테스트하려면 객체를 생성하는데 어려움이 있다.
중복 코드 또한 많이 발생해 Session을 생성할 때 생성자의 인자가 변경되는 경우 변경할 부분이 많아진다.
https://www.arhohuttunen.com/test-data-builders/ 문서 참고해 Session과 같이 복잡한 객체의 테스트 데이터를 생성할 때 어떤 방법을 사용할 것인지 선택해 보면 좋겠다.
이번 기회에 내가 선호하는 방법을 적용해 보고 앞으로도 쭈욱 활용하는 방식이면 좋겠다.
void 유료_강의_수강_신청_실패_수강_인원_초과_테스트() { | ||
assertThatIllegalArgumentException() | ||
.isThrownBy(() -> paidSession.isPossibleToRegister(40_000L, 30)); | ||
} |
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.
session.enroll(nsUser);
와 같은 메시지를 보내면 session이 수강 신청 가능 여부룰 확인하고, 수강 신청이 가능하면 수강생 목록에 추가하는 방식으로 구현해 보면 어떨까?
2단계임에도 불구하고 벌써 db까지 고려하다보니 도메인 로직 구현에 집중하는데 어려움이 있는 느낌이다.
db를 고려하지 않고, 앞의 로또, 사다리와 같은 관점에서 도메인 객체 설계와 로직 구현에 집중하면 어떨까?
미션 수행 내용
확인 부탁드립니다.