-
Notifications
You must be signed in to change notification settings - Fork 248
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Step3 #355
base: yumble
Are you sure you want to change the base?
Step3 #355
Changes from all commits
96a1d78
7624a6e
ede2296
dd535f1
9da12f1
a5ce8e4
e2476c7
284bba0
2ccb9a4
ba41c02
383e2f3
8287934
b4320d3
b05446e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,10 @@ | ||
package nextstep.courses.domain; | ||
|
||
import java.util.List; | ||
import nextstep.courses.exception.SessionException; | ||
import nextstep.courses.exception.SessionException.SessionFeeNotEqualException; | ||
import nextstep.courses.exception.SessionException.SessionFullException; | ||
import nextstep.courses.exception.SessionException.SessionNotOpenException; | ||
import nextstep.users.domain.NsUser; | ||
|
||
public class Session { | ||
|
@@ -10,55 +14,51 @@ public class Session { | |
private final Thumbnail thumbnail; | ||
private final SessionType sessionType; | ||
private final SessionStatus sessionStatus; | ||
private final List<NsUser> students; | ||
|
||
public Session(Long sessionId, String sessionName, Period sessionPeriod, Thumbnail thumbnail, | ||
SessionType sessionType, SessionStatus sessionStatus, List<NsUser> students) { | ||
SessionType sessionType, SessionStatus sessionStatus) { | ||
this.sessionId = sessionId; | ||
this.sessionName = sessionName; | ||
this.sessionPeriod = sessionPeriod; | ||
this.thumbnail = thumbnail; | ||
this.sessionType = sessionType; | ||
this.sessionStatus = sessionStatus; | ||
this.students = students; | ||
} | ||
|
||
public boolean isEnrollmentPossible(Integer sessionFee) { | ||
public boolean isEnrollmentPossible(SessionStudents students, Integer sessionFee) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 저는 요 부분이 추상화가 되는게 맞지 않나 생각합니다..! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 그리고 요거 enroll할때 내부에서 유효성검사를 하는식으로 해야되지 않을까요 ? 지금은 외부에서 판단하도록 되어있는 것 같아서요! |
||
if (!isRecruiting()) { | ||
return false; | ||
throw new SessionNotOpenException("강의가 모집중인 상태가 아닙니다."); | ||
} | ||
if (!isWithinCapacity()) { | ||
return false; | ||
if (!isWithinCapacity(students)) { | ||
throw new SessionFullException("강의의 수용인원이 다 찼습니다."); | ||
} | ||
if (!checkSessionFeeEquality(sessionFee)) { | ||
return false; | ||
throw new SessionFeeNotEqualException("접수하신 수강료가 강의 수강료와 일치하지 않습니다."); | ||
} | ||
return true; | ||
} | ||
boolean isRecruiting() { | ||
protected boolean isRecruiting() { | ||
return sessionStatus == SessionStatus.RECRUITING; | ||
} | ||
|
||
boolean isWithinCapacity() { | ||
return this.sessionType.isWithinCapacity(this.students.size()); | ||
protected boolean isWithinCapacity(SessionStudents students) { | ||
return students.isWithinCapacity(this.sessionType); | ||
} | ||
|
||
boolean checkSessionFeeEquality(Integer sessionFee) { | ||
protected boolean checkSessionFeeEquality(Integer sessionFee) { | ||
return this.sessionType.checkSessionFeeEquality(sessionFee); | ||
} | ||
|
||
public void enroll(NsUser student) { | ||
this.students.add(student); | ||
public Student enroll(SessionStudents students, NsUser user) { | ||
Student student = new Student(this.sessionId, user.getId()); | ||
students.enroll(student); | ||
return student; | ||
} | ||
|
||
public Long getSessionId() { | ||
return sessionId; | ||
} | ||
|
||
public List<NsUser> getStudents() { | ||
return students; | ||
} | ||
|
||
@Override | ||
public String toString() { | ||
return "Session{" + | ||
|
@@ -68,7 +68,6 @@ public String toString() { | |
", thumbnail=" + thumbnail + | ||
", sessionType=" + sessionType + | ||
", sessionStatus=" + sessionStatus + | ||
", students=" + students + | ||
'}'; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
package nextstep.courses.domain; | ||
|
||
public interface SessionRepository { | ||
Session findById(Long id); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
package nextstep.courses.domain; | ||
|
||
import java.util.List; | ||
|
||
public class SessionStudents { | ||
|
||
private final List<Student> students; | ||
|
||
public SessionStudents(List<Student> students) { | ||
this.students = students; | ||
} | ||
|
||
public void enroll(Student student) { | ||
students.add(student); | ||
} | ||
|
||
public boolean isWithinCapacity(SessionType sessionType) { | ||
return sessionType.isWithinCapacity(students.size()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sessionType을 가져와서 검증하는게 아니라 sessionStudents가 현재 수강인원을 제공해주는게 더 자연스럽지 않을까요 ? |
||
} | ||
|
||
public List<Student> getStudents() { | ||
return students; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
package nextstep.courses.domain; | ||
|
||
public interface SessionStudentsRepository { | ||
int save(Student student); | ||
|
||
SessionStudents findBySessionId(Long sessionId); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,20 +1,21 @@ | ||
package nextstep.courses.domain; | ||
|
||
public abstract class SessionType { | ||
public interface SessionType { | ||
|
||
private final boolean isPaid; | ||
|
||
public SessionType(boolean isPaid) { | ||
this.isPaid = isPaid; | ||
static SessionType determineSessionType(boolean isPaid, Long sessionTypeId, Integer maxStudents, Integer sessionFee) { | ||
if (isPaid) { | ||
return new PaidSession(sessionTypeId, maxStudents, sessionFee); | ||
} | ||
return new FreeSession(sessionTypeId); | ||
} | ||
|
||
public static SessionType determineSessionType(boolean isPaid, Integer maxStudents, Integer sessionFee) { | ||
if (isPaid) { | ||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. SessionType의 구현체들이라 Type을 명시해주던가 다른 이름을 주는게 좋을 것 같아요 그리고 얘는 validateEnrollment 정도만 추상화를 하고, 내부에서 뭘 하는지는 각각의 구현체에 맡기고, 이 데이터들을 필드로 가지고 있는게 아니라 검증 할 때 파라미터로 받으면 되지 않을까요 ? 지금 밑에 있는 두개 메서드는 Free일때는 사실 아예 없어도 될 것 처럼 보여서요! |
||
} | ||
return new FreeSession(isPaid); | ||
return new FreeSession(freeSessionTypeId); | ||
} | ||
|
||
public abstract boolean isWithinCapacity(Integer size); | ||
public abstract boolean checkSessionFeeEquality(Integer sessionFee); | ||
boolean isWithinCapacity(Integer size); | ||
boolean checkSessionFeeEquality(Integer sessionFee); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
package nextstep.courses.domain; | ||
|
||
import java.util.Objects; | ||
|
||
public class Student { | ||
|
||
private Long id; | ||
private final Long sessionId; | ||
private final Long studentId; | ||
|
||
public Student(Long sessionId, Long studentId) { | ||
this(null, sessionId, studentId); | ||
} | ||
|
||
public Student(Long id, Long sessionId, Long studentId) { | ||
this.id = id; | ||
this.sessionId = sessionId; | ||
this.studentId = studentId; | ||
} | ||
|
||
public Long getSessionId() { | ||
return sessionId; | ||
} | ||
|
||
@Override | ||
public boolean equals(Object o) { | ||
if (this == o) { | ||
return true; | ||
} | ||
if (o == null || getClass() != o.getClass()) { | ||
return false; | ||
} | ||
Student student = (Student) o; | ||
return Objects.equals(sessionId, student.sessionId) | ||
&& Objects.equals(studentId, student.studentId); | ||
} | ||
|
||
@Override | ||
public int hashCode() { | ||
return Objects.hash(sessionId, studentId); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
package nextstep.courses.domain; | ||
|
||
public interface ThumbnailRepository { | ||
Thumbnail findById(Integer id); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
package nextstep.courses.exception; | ||
|
||
public class SessionException extends IllegalArgumentException{ | ||
public SessionException() { | ||
} | ||
|
||
public SessionException(String message) { | ||
super(message); | ||
} | ||
public static class SessionNotOpenException extends SessionException { | ||
public SessionNotOpenException(String message) { | ||
super(message); | ||
} | ||
} | ||
public static class SessionFullException extends SessionException { | ||
public SessionFullException(String message) { | ||
super(message); | ||
} | ||
} | ||
|
||
public static class SessionFeeNotEqualException extends SessionException { | ||
public SessionFeeNotEqualException(String message) { | ||
super(message); | ||
} | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아 세션타입을 구현한게 세션이었군요 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DB 맵핑을 더 직관적으로 표현한다면, SessionType을 enum으로 만들고 DB에 업데이트 하는 정보를 enum을 넣으면 어떨까 싶네요!
@Enumerated
를 String 타입으로 하는 것 처럼요!