Step2 : 수강신청(도메인 모델) #805
Step2 : 수강신청(도메인 모델) #805
Conversation
javajigi
left a comment
javajigi
left a comment
There was a problem hiding this comment.
2단계는 역시나 테이블과 매핑하지 않아 작은 단위의 여러 객체로 분리 👍
전체적인 객체 설계와 단위 테스트 구현 잘 했네요.
그럼에도 불구하고 한번 더 고민해 봤으면 하는 부분이 있어 피드백 남겨요.
Q1. 무료인 경우와 유료인 경우를 구분하는 것인데요
특히 무료인 경우 수강료와 정원을 0 으로 두어야할지 고민이 되었습니다.
0 으로 두기에는 객체에 대한 도메인을 오염시킨다는 생각이 들었습니다.
그래서 null 이라는 것이 프로그램적으로는 위험할 수 있지만, 의미 그대로 존재하지 않는다 생각해서 null 로 이용했습니다
혹시 이렇게 접근하는게 괜찮을까요?
A1. null로 구현하기보다 null을 추상화한 Null Object를 추가해 보는 것은 어떨까요?
AI에게 Null Object에 대해 질문해 보고 피드백을 받아 적용해 볼 것을 추천합니다.
Q2. 무료 유료를 구분하면서 ProvidePolicy 를 초기화 하는데 new ProvidePolicy() 빈 생성자인 경우를 무료로 정의했습니다.
유료인 경우는 new ProvidePolicy(x, y) 인자가 있는 경우로 정의 했습니다.
이렇게 접근하는 것이 개발하는 과정에서 계속 불편했는데요 이렇게 접근이 괜찮을까요?
아니면 고민하다가 전략패턴으로 도전해볼까 고민했는데, 오버엔지니어링이라 생각들어서 일단 시도 하지 않았습니다.
A2. 무료 유료 구분에 따른 구분은 ProvideType 값에 따라 결정되어야 하지 않을까요?
생성자 유무로 판단하는 것은 적절해 보이지 않은 것 같아요.
Q3. 상위 객체 메서드부터 여러 단계의 하위 객체의 메서드까지 파라미터 터널링 이 몇부분 보이는데요 이부분도 걱정이됩니다.
A3. 피드백에도 남겼는데요.
상위 객체의 필드도 private으로 구현하고 메서드를 통해 접근할 것을 추천합니다.
|
|
||
| public void apply(long userId, long sessionId, Payment payment) throws CanNotJoinException { | ||
| Session session = sessions.findToApplySession(sessionId); | ||
| if(payment == null) { |
There was a problem hiding this comment.
payment 값에 따라 결정하기보다 Session의 ProvideType에 따라 결정되도록 해야하지 않을까?
| import nextstep.courses.enumerate.ProvideType; | ||
| import nextstep.payments.domain.Payment; | ||
|
|
||
| public class ProvidePolicy { |
There was a problem hiding this comment.
수강 신청 가능 여부를 판단할 수 있는 모든 값을 가지도록 구현하면 어떨까?
예를 들어 SessionStatus, 현재 수강신청한 수강생 목록도 수강 신청 여부를 판단하는데 필요한 값으로 판단할 수 있지 않을까?
|
|
||
| static { | ||
| try { | ||
| freeSession = new Session( |
There was a problem hiding this comment.
Session 객체와 같이 인스턴스 변수가 많은 객체를 테스트하려면 객체를 생성하는데 어려움이 있다.
중복 코드 또한 많이 발생해 Session을 생성할 때 생성자의 인자가 변경되는 경우 변경할 부분이 많아진다.
https://www.arhohuttunen.com/test-data-builders/ 문서 참고해 Session과 같이 복잡한 객체의 테스트 데이터를 생성할 때 어떤 방법을 사용할 것인지 선택해 보면 좋겠다.
이번 기회에 내가 선호하는 방법을 적용해 보고 앞으로도 쭈욱 활용하는 방식이면 좋겠다.
1. Base를 추상 클래스로 구현하여 상속전용으로만 하고 직접 생성은 자식이 못하게 막음 (도메인 객체처럼 사용할 우려 막음) 2. Base 의 필드를 private 로 변경해 직접 수정 못하게 막음
2 participants
안녕하세요? 리뷰요청드립니다
프로그래밍 요구사항대로 하면서 점점 실무와 연결되는 느낌이 있었습니다! 좋은 과정 감사드립니다
요구조건이 몇 부분 모호해서 그런지(실무도 사실 그렇죠 😂) 개발하면서 생각보다 많은 시간이 소요되었네요
일단은 한번이라도 더 리뷰 요청드리고 다시 리팩터링 하는것이 바람직하다고 생각들어서 요청드립니다
다만 개발을 하면서 가장 고민되는 부분이 있었습니다.
특히 무료인 경우 수강료와 정원을 0 으로 두어야할지 고민이 되었습니다.
0 으로 두기에는 객체에 대한 도메인을 오염시킨다는 생각이 들었습니다.
그래서 null 이라는 것이 프로그램적으로는 위험할 수 있지만, 의미 그대로 존재하지 않는다 생각해서 null 로 이용했습니다
혹시 이렇게 접근하는게 괜찮을까요?
ProvidePolicy를 초기화 하는데new ProvidePolicy()빈 생성자인 경우를 무료로 정의했습니다.유료인 경우는
new ProvidePolicy(x, y)인자가 있는 경우로 정의 했습니다.이렇게 접근하는 것이 개발하는 과정에서 계속 불편했는데요 이렇게 접근이 괜찮을까요?
아니면 고민하다가 전략패턴으로 도전해볼까 고민했는데, 오버엔지니어링이라 생각들어서 일단 시도 하지 않았습니다.
파라미터 터널링이 몇부분 보이는데요 이부분도 걱정이됩니다.감사합니다 👍