Conversation
jinyoungchoi95
left a comment
There was a problem hiding this comment.
안녕하세요 유라님 😄
미션 잘 진행해주셨네요! 몇가지 코멘트 남겨두었으니 확인부탁드려요 :)
궁금하거나 고민이 되는 부분이 있으시다면 언제든 pr 코멘트 또는 dm으로 요청 부탁드립니다.
감사합니다 🙇♂️
|
|
||
| private final int width; | ||
| private final int height; | ||
| private final int sizeInBytes; |
There was a problem hiding this comment.
| private final int sizeInBytes; | |
| private final int sizeInBytes; |
👀
| if (width * 2 != height * 3) { | ||
| throw new InvalidCoverImageException("비율은 3:2여야 합니다."); | ||
| } | ||
| if (extension == null) { |
There was a problem hiding this comment.
null인 케이스도 있겠지만 확장자가 ImageExtension에 포함되지 않는 경우도 예외처리되면 좋겠네요!
|
|
||
| import java.util.List; | ||
|
|
||
| public class Sessions { |
| private List<Session> sessions; | ||
|
|
||
| public Sessions() { | ||
| } |
There was a problem hiding this comment.
| private List<Session> sessions; | |
| public Sessions() { | |
| } | |
| private final List<Session> sessions; | |
| public Sessions() { | |
| sessions = new ArrayList<>(); | |
| } |
이렇게 보완이 되어야할 것 같네요 😄
| throw new CannotEnrollSessionException("수강 조건이 맞지 않습니다."); | ||
| } | ||
|
|
||
| currentEnrolledCount++; |
There was a problem hiding this comment.
수강 수를 넣을 수도 있지만 누가 들었는지 수강생을 저장해보는건 어떨까요? NsUser를 활용해보면 좋을 것 같아요 :)
| @@ -0,0 +1,5 @@ | |||
| package nextstep.payments.domain; | |||
|
|
|||
| public interface EnrollmentPolicy { | |||
| private final LocalDate endDate; | ||
| private final CoverImage coverImage; | ||
| private final SessionStatus status; | ||
| private final EnrollmentPolicy enrollmentPolicy; |
There was a problem hiding this comment.
(반영하지 않으셔도 됩니다.) Session이라는 큰 추상적 표현을 생각해보면 Session을 인터페이스로 두고, "유료강의"와 "무료강의"라는 구현체를 두는 형태를 고민해볼 수 있을 것 같아요 😄
| public boolean canEnroll(int currentEnrolledCount, Payment payment) { | ||
| return payment.isSameAmount(0L); | ||
| } |
There was a problem hiding this comment.
어떻게보면 무료 강의는 비교 구문 자체가 필요없지 않을까요? 👀
| public class PaidEnrollmentPolicy implements EnrollmentPolicy { | ||
|
|
||
| private final int maxEnrolledCount; | ||
| private final int price; |
There was a problem hiding this comment.
가격은 어떻게보면 Session이 들고 있어야하는 정보가 아닐까요?
| import static org.junit.jupiter.api.Assertions.assertFalse; | ||
| import static org.junit.jupiter.api.Assertions.assertTrue; | ||
|
|
||
| public class EnrollmentPolicyTest { |
There was a problem hiding this comment.
테스트코드를 작성할 때 EnrollmentPolicy 하위에 있는 구현체들에 대해 EnrollmentPolicyTest로 몰아넣을수도 있지만, 테스트하는 대상은 각 구현체들로 구분지어져있기 때문에 개인적으로는 FreeEnrollmentPolicyTest, PaidEnrollmentPolicyTest와 같이 클래스별로 테스트 클래스를 만드는 것을 선호합니다 😄
안녕하세요!
각 클래스 설명입니다.
Course - 과정
Session - 강의
Sessions - 강의 일급 객체
SessionStatus - 강의 상태 : 준비중, 모집중, 종료
CoverImage - 강의 커버 이미지
ImageExtension - 이미지 확장자
EnrollmentPolicy - 강의 정책 인터페이스로 등록 가능한지 판단함
PaidEnrollmentPolicy - 유료 강의 정책
FreeEnrollmentPolicy - 무료 강의 정책
감사합니다:)