Lms step2#126
Conversation
- Answer에서 setter 메서드 제거 - Answer에서 toString() 함수 제일 밑으로 이동
- Session에 Set<Student> 추가 - StudentTest 위한SessionCreator 추가 - Student 객체 구현
- 테스트 추가 - Student에 로직 추가
javajigi
left a comment
javajigi
left a comment
There was a problem hiding this comment.
도메인 객체 분리, 도메인에 수강신청 관련 로직 구현, 단위 테스트 코드 구현 💯
지금 구현한 객채 설계도 좋은데요.
Session 객체를 더 작은 단위의 객체로 분리하고, 책임을 분리해 보면 좋을 것 같아 피드백 남겨 봅니다.
| - 강의는 무료 강의와 유료 강의로 나뉜다. | ||
| - 강의 상태는 준비중, 모집중, 종료 3가지 상태를 가진다. | ||
| - 강의 수강신청은 강의 상태가 모집중일 때만 가능하다. | ||
| - 강의는 강의 최대 수강 인원을 초과할 수 없다. No newline at end of file |
|
|
||
| private Long creatorId; | ||
|
|
||
| private final List<Session> sessions = new ArrayList<>(); |
| private final SessionStatus sessionStatus; | ||
| private final SessionTimeLine sessionTimeLine; | ||
| private final Set<Student> students = new HashSet<>(); | ||
| private final Long maxNumberOfStudent; |
There was a problem hiding this comment.
수강 신청 로직이 가능한지의 여부를 판단하기 위해 sessionStatus, students, maxNumberOfStudent 3개의 값을 활용해야 한다.
또한 3개의 값은 서로 연관되어 있는 값이다.
이 3개의 값을 새로운 객체로 분리하고, 수강신청 로직을 이 객체가 담당하도록 구현하는 것은 어떨까?
| this.maxNumberOfStudent = maxNumberOfStudent; | ||
| } | ||
|
|
||
| public void add(Student student) { |
There was a problem hiding this comment.
메서드 이름을 add로 사용하면 collection에 값을 추가하는 느낌이다.
수강신청을 의미하는 이름으로 변경하는 것은 어떨까?
| if (!sessionStatus.canJoin()) { | ||
| throw new CannotEnrollException("현재는 수강신청을 할 수 없는 강의 상태입니다. 현재 강의 상태 = " + sessionStatus.name()); | ||
| } | ||
|
|
||
| if (isPositionFull()) { | ||
| throw new CannotEnrollException( | ||
| "현재 강의(Session)는 수강인원이 꽉 차서 더 이상 등록할 수 없습니다." + "최대인원 = " + maxNumberOfStudent); | ||
| } | ||
| this.students.add(student); |
There was a problem hiding this comment.
수강 신청 로직을 앞에서 분리한 sessionStatus, students, maxNumberOfStudent 3개의 값을 가지는 객체가 담당하도록 구현하는 것은 어떨까?
| @@ -0,0 +1,18 @@ | |||
| package nextstep.courses.domain; | |||
|
|
|||
| public class SessionInfo { | |||
| DeleteHistories deleteHistories = new DeleteHistories(); | ||
| deleteHistories.add(DeleteHistory.createQuestion(this)); | ||
|
|
||
| return answers.delete(deleteHistories); |
|
|
||
| public class SessionCreator { | ||
|
|
||
| public static Session create(Long maxNumberOfStudent, SessionStatus sessionStatus) { |
- add -> enroll
- SessionEnrollment 객체 구현 완료 - SessionEnrollmentTest 테스트 추가 - Session 에서 강의 등록에 관련 있는 3개의 멤버변수를 빼서 SessionEnrollment로 분리 완료
|
@javajigi
아래 코드에서 Session 내 SessionEnrollment 객체가 있는데요 public Long totalStudentNum() {
return sessionEnrollment.totalStudentNum();
}이렇게 되면 Session에서 totalStudentNum() 를 테스트를 하는 것(A) 과 sessionEnrollment의 totalStudentNum() 테스트를 하는 것은 중복이 될 수도 있다고 생각하는데요. 어떻게 생각하시나요? 아 그리고 감사합니다. |
- 이미 등록을 한 학생인 경우 예외를 던진다. - README.md 추가 - AlreadyEnrollmentException 추가 - 테스트 추가
javajigi
left a comment
javajigi
left a comment
There was a problem hiding this comment.
피드백 반영 넘 잘 했네요. 💯
미션 끝까지 마무리 도전해 보시죠.
2 participants
2단계 리뷰 부탁드립니다. 감사합니다.
이전 단계 PR(Lms step1 #80) 에서 답글 확인 부탁드립니다!
보통 equals와 hashCode 오버라이딩은 대부분 하는 편인가요? 객체지향적으로 개발한다면 객체 동등성 비교를 위해서는 해야하는 듯한데, 필요한 경우가 아니라면 안했었기 때문에 더더욱 궁금하네요.