Step3 #355
Step3 #355
Conversation
- FreeSession, PaidSession 분리해서 테이블 만듦
|
a,b가 어떤 차이가 있는걸까요 ? 말씀하신 방법들 다 방안이 될 수 있다고 생각해요. 하지만 c의 경우는 SessionType 별 분기되는 로직이 있기때문에 타입을 특정할 수 있는 정보는 있어야 할 것 같구요. 오히려 maxStudents나 sessionFee를 sessionsType이 책임지지 않게 하는게 맞을 것 같아요 예를 들면 A 강의와 B 강의 둘 다 유료강의인데 서로 다른 가격과 수강인원을 가지고 있다고 하면 sessionType에 묶이면 표현이 안 될 것 같아요..! |
|
a번의 방법은 DB상에서 테이블을 Free table, Paid table로 분기를 했다는 걸 의미했던거고,
|
| @@ -1,13 +1,14 @@ | |||
| package nextstep.courses.domain; | |||
|
|
|||
| public class PaidSession extends SessionType{ | |||
| public class PaidSession implements SessionType{ | |||
There was a problem hiding this comment.
DB 맵핑을 더 직관적으로 표현한다면, SessionType을 enum으로 만들고 DB에 업데이트 하는 정보를 enum을 넣으면 어떨까 싶네요! @Enumerated 를 String 타입으로 하는 것 처럼요!
| } | ||
|
|
||
| public boolean isEnrollmentPossible(Integer sessionFee) { | ||
| public boolean isEnrollmentPossible(SessionStudents students, Integer sessionFee) { |
| } | ||
|
|
||
| public boolean isEnrollmentPossible(Integer sessionFee) { | ||
| public boolean isEnrollmentPossible(SessionStudents students, Integer sessionFee) { |
There was a problem hiding this comment.
그리고 요거 enroll할때 내부에서 유효성검사를 하는식으로 해야되지 않을까요 ? 지금은 외부에서 판단하도록 되어있는 것 같아서요!
| } | ||
|
|
||
| public boolean isWithinCapacity(SessionType sessionType) { | ||
| return sessionType.isWithinCapacity(students.size()); |
There was a problem hiding this comment.
sessionType을 가져와서 검증하는게 아니라 sessionStudents가 현재 수강인원을 제공해주는게 더 자연스럽지 않을까요 ?
| return new PaidSession(isPaid, maxStudents, sessionFee); | ||
| static SessionType determineSessionTypeByDB(Long freeSessionTypeId, Long paidSessionTypeId, Integer maxStudents, Integer sessionFee) { | ||
| if (freeSessionTypeId == null) { | ||
| return new PaidSession(paidSessionTypeId, maxStudents, sessionFee); |
There was a problem hiding this comment.
SessionType의 구현체들이라 Type을 명시해주던가 다른 이름을 주는게 좋을 것 같아요
개인적으로는 EnrollmentStrategy 같은걸로 만들면 되지 않을까 싶네요..!
그리고 얘는 validateEnrollment 정도만 추상화를 하고, 내부에서 뭘 하는지는 각각의 구현체에 맡기고, 이 데이터들을 필드로 가지고 있는게 아니라 검증 할 때 파라미터로 받으면 되지 않을까요 ?
지금 밑에 있는 두개 메서드는 Free일때는 사실 아예 없어도 될 것 처럼 보여서요!
DB연계를 생각하면서 하다보니 생각보다 많이 막힌 것 같아요ㅜㅜ
SessionType을 쿼리로 가져오려니 코드가 깔끔하다고 생각이 들지않아 리팩터링이 필요할 것 같아 리뷰어님 생각을 여쭤봅니다.. 제가 생각했던 방안은 3가지 정도였습니다.
세션의 필드였던 List students를 session과 nsUser간의 다대다 관계라고 생각해서 student라는 클래스로 분리를 진행했습니다.