Step2 #288
Step2 #288
Conversation
| public Course() { | ||
| } | ||
|
|
||
| public Course(List<Session> sessions) { |
There was a problem hiding this comment.
모든 인자를 받는 생성자에도 List<Session> sessions를 추가하는 건 어떨까요? :)
현재 이 생성자는 테스트에서만 사용되어서, 테스트만을 위해 생성자를 추가한 것처럼 보이기도 하네요.
| public Session() { | ||
| } | ||
|
|
||
| public Session(int tuition) { |
There was a problem hiding this comment.
인자를 하나씩 받는 생성자를 종류별로 만드신 이유가 있을까요?
특정 값을 인자로 받는 생성자가 제공된다는 것은, 해당 값만 있어도 객체를 생성할 수 있다는 의미라고 생각합니다.
image만 존재하고 startDate, endDate는 없는 Session이 만들어질 수도 있을까요?
| this.endDate = endDate; | ||
| } | ||
|
|
||
| public SessionStatus isSessionStatus() { |
There was a problem hiding this comment.
is는 보통 boolean 값을 반환하는 메서드의 prefix 로 사용합니다. 예를 들면, isEmpty와 같은 메서드가 있습니다.
sessionStatus를 반환하는 메서드이니 getSessionStatus는 어떨까요? 한걸음 더 나아가서, 값을 꺼내는 메서드가 필요할 때 제공하는 건 어떨까요? 어쩌면 getSessionStatus는 필요 없고, isReady와 같이 세션의 상태를 물어보는 메서드로 충분할 수도 있습니다.
| return sessionStatus; | ||
| } | ||
|
|
||
| public void addStudent() { |
| Course course = new Course(sessions); | ||
| assertThat(course.getSessions()).isNotEmpty(); | ||
| } | ||
| } |
There was a problem hiding this comment.
파일 마지막에 newline 이 없다고 깃헙에서 경고를 띄워주네요.
파일 마지막의 newline 은 무슨 역할을 하는지 학습해보신 후, IDE 설정을 통해 휴먼 에러를 방지해보시면 좋겠습니다.
| } | ||
|
|
||
| private static void validateImage(String type, int width, int height) { | ||
| if (width * height > 1024 * 1024) { |
There was a problem hiding this comment.
1 픽셀을 표현하기 위해 사용되는 바이트 수가 많아진다면, 크기만으로 검증하는 건 어려울 것 같아요.
사이즈 필드를 명시적으로 추가하고 검증에 사용하는 건 어떨까요?
| throw new IllegalArgumentException("이미지 크기는 1MB 이하여야 한다."); | ||
| } | ||
|
|
||
| if (!(type.contains("gif") || type.contains("jpg") || type.contains("jpeg") || type.contains("png") || type.contains("svg"))) { |
There was a problem hiding this comment.
타입 검증 좋습니다. 👍
우리가 관리하고 싶은 type이 정해져 있으니, enum 을 이용하는 방법도 사용해 볼 수 있겠네요!
| throw new IllegalArgumentException("이미지 타입은 gif, jpg(jpeg 포함),, png, svg만 허용한다."); | ||
| } | ||
|
|
||
| if (width < 300) { |
There was a problem hiding this comment.
width, height를 관리하는 사이즈 클래스를 만들어보는 건 어떨까요?
| } | ||
|
|
||
| public Session(LocalDateTime startDate, LocalDateTime endDate) { | ||
| this.startDate = startDate; |
There was a problem hiding this comment.
시작일과 종료일에 대한 밸리데이션도 할 수 있을 것 같아요.
두 값이 반드시 존재한다거나, 종료일은 시작일 이후와 같은 조건이요.
세션을 생성할 때 필수가 되는 필드를 정의 후 이에 대한 생성자를 제공한다면, 이 생성자에서 모든 밸리데이션을 할 수 있겠네요!
There was a problem hiding this comment.
아직 많이 미숙하지만 tdd로 미션을 진행하다 보니 개별적으로 작성한 클래스들을 마지막에 합치는 과정에서 많은 어려움이 있었습니다.
처음부터 큰 그림 로직 설계를 한 다음 세부 클래스 tdd를 진행해야 할까요??
tdd 자체의 어려움과 새로 접한 도메인의 어려움이 섞여서 더 어려우셨을 것 같습니다.
먼저 관련해서 읽어보시면 도움이 될 것 같은 블로그를 공유드려요. 주요 키워드는 Inside Out 과 Outside In 입니다.
개별적으로 클래스를 작성한 후 합치는 방법은 초기 설계가 잘 되어 있거나 우리가 잘 아는 도메인일 때 더 유용할 수 있다고 생각합니다. 말씀하신 것처럼, 처음에 큰 로직(전체 설계)을 고민해보신 후 tdd로 접근하시는 것이 더 수월하셨을 것 같아요. :)
|
추가로 코멘트 남깁니다. :) 현재 서비스 로직의 구현이 없는데요. 과정 생성, 강의 생성, 수강 신청 정도의 기능은 구현해보시면 좋겠습니다. 프로그래밍 요구사항
|
아직 많이 미숙하지만 tdd로 미션을 진행하다 보니 개별적으로 작성한 클래스들을 마지막에 합치는 과정에서 많은 어려움이 있었습니다.
처음부터 큰 그림 로직 설계를 한 다음 세부 클래스 tdd를 진행해야 할까요??
1차 피드백 받기 위해 pr 요청합니다.