Skip to content
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

4단계 - 수강신청(요구사항 변경) #395

Open
wants to merge 6 commits into
base: hvoiunq
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions STEP4.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
## 변경된 기능 요구사항
강의 수강신청은 강의 상태가 모집중일 때만 가능하다.
* 강의가 진행 중인 상태에서도 수강신청이 가능해야 한다.
* 강의 진행 상태(준비중, 진행중, 종료)와 모집 상태(비모집중, 모집중)로 상태 값을 분리해야 한다.
강의는 강의 커버 이미지 정보를 가진다.
* 강의는 하나 이상의 커버 이미지를 가질 수 있다.
강사가 승인하지 않아도 수강 신청하는 모든 사람이 수강 가능하다.
* 우아한테크코스(무료), 우아한테크캠프 Pro(유료)와 같이 선발된 인원만 수강 가능해야 한다.
* 강사는 수강신청한 사람 중 선발된 인원에 대해서만 수강 승인이 가능해야 한다.
* 강사는 수강신청한 사람 중 선발되지 않은 사람은 수강을 취소할 수 있어야 한다.

## 프로그래밍 요구사항
* 리팩터링할 때 컴파일 에러와 기존의 단위 테스트의 실패를 최소화하면서 점진적인 리팩터링이 가능하도록 한다.
* DB 테이블에 데이터가 존재한다는 가정하에 리팩터링해야 한다.
* 즉, 기존에 쌓인 데이터를 제거하지 않은 상태로 리팩터링 해야 한다.
### 핵심 학습 목표
* DB 테이블이 변경될 때도 스트랭글러 패턴을 적용해 점진적인 리팩터링을 연습한다.
* 스트랭글러(교살자) 패턴 - 마틴 파울러
* 스트랭글러 무화과 패턴

## STEP4 기능분해
* 강의 수강신청은 강의 상태가 모집중일 때만 가능하다.
* [X] 강의가 진행중이어도 모집중이면 수강신청이 가능하다.
* [X] 강의가 진행중에 비모집중이면 수강신청 불가능하다는 Exception이 발생한다.
* [X] 강의가 준비중에 모집중이면 수강신청 가능하다.
* [X] 강의가 준비중이어도 비모집중이면 수강신청이 불가능하다는 Exception이 발생한다.
* 강의는 강의 커버 이미지 정보를 가진다.
* [X] 강의는 하나 이상의 커버 이미지를 갖는다.
* 강사가 승인하지 않아도 수강 신청하는 모든 사람이 수강 가능하다.
* [X] 수강신청한 사람 중 선발되지 않은 수강생은 수강 취소가 된다.
* [X] 수강신청한 사람 중 선발된 사람에 대해 수강 승인이 가능하다.

## STEP4 리팩토링
* [X] Session 인스턴스 변수 줄이기
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package nextstep.courses.domain;
package nextstep.courses.common;

import java.time.LocalDateTime;

Expand Down
1 change: 1 addition & 0 deletions src/main/java/nextstep/courses/domain/Course.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package nextstep.courses.domain;

import nextstep.courses.common.SystemTimeStamp;
import nextstep.courses.domain.session.Session;
import nextstep.courses.domain.session.Sessions;

Expand Down
32 changes: 28 additions & 4 deletions src/main/java/nextstep/courses/domain/Student.java
Original file line number Diff line number Diff line change
@@ -1,11 +1,35 @@
package nextstep.courses.domain;

import nextstep.courses.domain.session.Session;
import nextstep.courses.domain.session.RegistrationState;

public class Student {
private long id;

private long nsUserId;
private long sessionId;
private RegistrationState registrationState;

public Student(long nsUserId, long sessionId, RegistrationState registrationState) {
this.nsUserId = nsUserId;
this.sessionId = sessionId;
this.registrationState = registrationState;
}

public void isCanceled() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isCanceled 이름은 boolean 값이 반환되는 경우 사용하는 경향이 있다.
이 이름보다 cancel, approve와 같은 이름을 사용하는 것을 추천한다.

this.registrationState = RegistrationState.CANCELED;
}

public void isApproved() {
this.registrationState = RegistrationState.APPROVED;
}

public long getNsUserId() {
return nsUserId;
}

public long getSessionId() {
return sessionId;
}

private Session session;
public RegistrationState getRegistrationState() {
return registrationState;
}
}
9 changes: 2 additions & 7 deletions src/main/java/nextstep/courses/domain/image/SessionImage.java
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package nextstep.courses.domain.image;

import nextstep.courses.InvalidImageFormatException;
import nextstep.courses.domain.SystemTimeStamp;
import nextstep.courses.common.SystemTimeStamp;
import nextstep.courses.domain.session.Session;

import java.time.LocalDateTime;
Expand All @@ -19,15 +18,11 @@ public class SessionImage {


public static SessionImage valueOf(long id, Session session, int size, int width, int height, String imageType) {
return new SessionImage(id, "tmp", session.getSessionId()
return new SessionImage(id, "tmp", session.getId()
, new ImageFormat(size, width, height, ImageType.validateImageType(imageType))
, new SystemTimeStamp(LocalDateTime.now(), null));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

질문에 있어 피드백으로 남김
날짜 또한 테스트 입장에서 생각해보면 Random과 같이 테스트하기 어려운 코드이다.
그렇다고 현장에서 매번 주입받는 구조로 구현하는데는 한계가 있다.
따라서 LocalDateTime.now()를 기반으로 구현하고, 강의 시작일과 종료일과 같이 테스트가 필요한 경우에 대해서만 주입받는 형태로 구현하는 것은 어떨까?

}

public SessionImage(long id, String name, long sessionId, int size, int width, int height, ImageType imageType) {
this(id, name, sessionId, new ImageFormat(size, width, height, imageType), new SystemTimeStamp(LocalDateTime.now(), null));
}

public SessionImage(long id, String name, long sessionId, ImageFormat imageFormat, SystemTimeStamp systemTimeStamp) {
this.id = id;
this.name = name;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,8 @@
import java.util.function.BiFunction;

public enum EnrollmentStatus {
PREPARING("준비중"),
RECRUITING("모집중"),
CLOSE("종료");
CLOSE("비모집중");

private String status;

Expand Down
26 changes: 26 additions & 0 deletions src/main/java/nextstep/courses/domain/session/FreeSession.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package nextstep.courses.domain.session;

import nextstep.courses.common.SystemTimeStamp;
import nextstep.courses.domain.Student;

import java.time.LocalDate;
import java.time.LocalDateTime;

public class FreeSession extends Session {

public static FreeSession valueOf(long id, String title, long courseId, EnrollmentStatus enrollmentStatus
, LocalDate startDate, LocalDate endDate, LocalDateTime createdAt, LocalDateTime updatedAt) {
return new FreeSession(new SessionInfo(id, title, courseId, SessionType.FREE)
, new SessionPlan(enrollmentStatus, startDate, endDate)
, new SystemTimeStamp(createdAt, updatedAt));
}

public FreeSession(SessionInfo sessionInfo, SessionPlan sessionPlan, SystemTimeStamp systemTimeStamp) {
super(sessionInfo, sessionPlan, systemTimeStamp);
}

@Override
public void signUp(Student student) {
super.signUp(student);
}
}
35 changes: 22 additions & 13 deletions src/main/java/nextstep/courses/domain/session/PaidSession.java
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
package nextstep.courses.domain.session;

import nextstep.courses.CannotSignUpException;
import nextstep.courses.domain.Course;
import nextstep.courses.domain.SystemTimeStamp;
import nextstep.courses.common.SystemTimeStamp;
import nextstep.courses.domain.Student;
import nextstep.payments.domain.Payment;
import nextstep.users.domain.NsUser;

Expand All @@ -17,34 +17,43 @@ public static PaidSession feeOf(long id, String title, long courseId,
EnrollmentStatus enrollmentStatus, LocalDate startDate,
LocalDate endDate, LocalDateTime createdAt, LocalDateTime updatedAt,
int maxStudentCount, Long sessionFee) {
return new PaidSession(id, title, courseId,
return new PaidSession(new SessionInfo(id, title, courseId, SessionType.PAID),
new SessionPlan(enrollmentStatus, startDate, endDate),
new SystemTimeStamp(createdAt, updatedAt),
maxStudentCount, sessionFee);
}

public PaidSession(Long sessionId, String title, long courseId,
SessionPlan sessionPlan, SystemTimeStamp systemTimeStamp,
public PaidSession(SessionInfo sessionInfo, SessionPlan sessionPlan, SystemTimeStamp systemTimeStamp,
int maxStudentCount, Long sessionFee) {
super(sessionId, title, courseId, SessionType.PAID, sessionPlan, systemTimeStamp);
super(sessionInfo, sessionPlan, systemTimeStamp);
this.maxStudentCount = maxStudentCount;
this.sessionFee = sessionFee;
}

@Override
public void signUp(NsUser nsUser, Payment payment) throws CannotSignUpException {
validateAvailableSignUp();
validateSessionFeeMatchingPayment(payment);
super.signUp(nsUser, payment);
public void signUp(Student student) {
validateAvailableStudentCount();
validatePayInfo(student, getPayInfo(student));
super.signUp(student);
}

private void validateSessionFeeMatchingPayment(Payment payment) throws CannotSignUpException {
if (payment.isNotSamePrice(sessionFee)) {
private Payment getPayInfo(Student student) {
return Payment.paidOf("tmp", super.getId(), student.getNsUserId(), this.sessionFee); // 결제가 완료됐다고 가정하기 위함.
}

private void validatePayInfo(Student student, Payment payment) {
if (payment.getSessionId() != this.getId()) {
throw new CannotSignUpException("해당 강의 결제이력이 없습니다.");
}
if (student.getNsUserId() != payment.getNsUserId()) {
throw new CannotSignUpException("결제자와 신청자의 정보가 일치하지 않습니다.");
}
if (payment.isNotSameSessionFee(sessionFee)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 로직 구현을 보면 payment의 값을 꺼내고 있다.
값을 꺼내기 보다 메시지를 구현하는 방식으로 구현할 수 있는 방법은 어떨까?

throw new CannotSignUpException("결제금액과 수강료가 일치하지 않습니다.");
}
}

private void validateAvailableSignUp() throws CannotSignUpException {
private void validateAvailableStudentCount() throws CannotSignUpException {
if (maxStudentCount == super.getStudentCount()) {
throw new CannotSignUpException("최대 수강 인원을 초과했습니다.");
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package nextstep.courses.domain.session;

public enum RegistrationState {
PENDING, // 대기중
APPROVED, // 승인
CANCELED // 취소
}
96 changes: 49 additions & 47 deletions src/main/java/nextstep/courses/domain/session/Session.java
Original file line number Diff line number Diff line change
@@ -1,89 +1,92 @@
package nextstep.courses.domain.session;

import nextstep.courses.CannotSignUpException;
import nextstep.courses.domain.SystemTimeStamp;
import nextstep.courses.common.SystemTimeStamp;
import nextstep.courses.domain.Student;
import nextstep.courses.domain.image.SessionImage;
import nextstep.payments.domain.Payment;
import nextstep.users.domain.NsUser;
import nextstep.qna.NotFoundException;

import java.time.LocalDate;
import java.time.LocalDateTime;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.stream.Collectors;

public class Session {
private long sessionId;
private String title;
import static nextstep.courses.domain.session.RegistrationState.*;

private long courseId;
private SessionType sessionType;
private List<NsUser> students;
private SessionImage sessionImage;
public class Session {
private SessionInfo sessionInfo;
private List<SessionImage> sessionImage;
private List<Student> students;
Comment on lines +17 to +18
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

위 2개의 List Collection 중 일급 콜렉션으로 구현하면 의미있는 필드가 있을까?
의미 있다 생각하는 필드를 일급 콜렉션으로 구현해 보는 것은 어떨까?

private SessionPlan sessionPlan;
private SystemTimeStamp systemTimeStamp;

public static Session valueOf(long id, String title, long courseId, EnrollmentStatus enrollmentStatus
, LocalDate startDate, LocalDate endDate, LocalDateTime createdAt, LocalDateTime updatedAt) {
return new Session(id, title, courseId, SessionType.FREE
, new SessionPlan(enrollmentStatus, startDate, endDate)
, new SystemTimeStamp(createdAt, updatedAt));
}

public Session(Long sessionId, String title, long courseId, SessionType sessionType, SessionPlan sessionPlan, SystemTimeStamp systemTimeStamp) {
this.sessionId = sessionId;
this.title = title;
this.courseId = courseId;
public Session(SessionInfo sessionInfo, SessionPlan sessionPlan, SystemTimeStamp systemTimeStamp) {
this.sessionInfo = sessionInfo;
this.students = new ArrayList<>(Collections.emptyList());
this.sessionType = sessionType;
this.sessionPlan = sessionPlan;
this.sessionImage = null;
this.sessionImage = new ArrayList<>(Collections.emptyList());;
this.systemTimeStamp = systemTimeStamp;
}

public void signUp(NsUser student, Payment payment) {
validateSessionStatus();
validatePayInfo(student, payment);
public void signUp(Student student) {
validateEnrollmentStatus();
students.add(student);
}

private void validatePayInfo(NsUser student, Payment payment) {
if (payment.getSessionId() != sessionId) {
throw new CannotSignUpException("결제한 강의정보가 맞지 않습니다.");
}
if (student.getId() != payment.getNsUserId()) {
throw new CannotSignUpException("결제자와 신청자의 정보가 일치하지 않습니다.");
}
}

private void validateSessionStatus() {
private void validateEnrollmentStatus() {
if (!EnrollmentStatus.canSignUp(this.sessionPlan.getEnrollmentStatus())) {
throw new CannotSignUpException("강의 모집중이 아닙니다.");
}
}

public void saveImage(SessionImage sessionImage) {
this.sessionImage = sessionImage;
this.sessionImage.add(sessionImage);
}

public void cancelStudent(Student student) {
Student validateStudent = validateIsAStudent(student);
validateStudent.isCanceled();
}

public void approveStudent(Student student) {
Student validatedStudent = validateIsAStudent(student);
validatedStudent.isApproved();
}

private Student validateIsAStudent(Student student) {
return this.getAllStudents().stream()
.filter(x -> x.getNsUserId() == student.getNsUserId())
.findFirst()
.orElseThrow(NotFoundException::new);
}

public int getStudentCount() {
return students.size();
}

public Long getSessionId() {
return sessionId;
public Long getId() {
return sessionInfo.getId();
}

public long getCourseId() {
return courseId;
return sessionInfo.getCourseId();
}

public String getTitle() {
return title;
return sessionInfo.getTitle();
}

public SessionType getSessionType() {
return sessionType;
return sessionInfo.getSessionType();
}

public List<Student> getStudents() {
return students.stream()
.filter(student -> student.getRegistrationState() == RegistrationState.APPROVED)
.collect(Collectors.toList());
}
public List<Student> getAllStudents() {
return students;
}

public SessionPlan getSessionPlan() {
Expand All @@ -94,8 +97,7 @@ public SystemTimeStamp getSystemTimeStamp() {
return systemTimeStamp;
}

public boolean hasImage() {
return !(sessionImage == null);
public int getImageCount() {
return sessionImage.size();
}

}
Loading