Skip to content

Step2 #721

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

Open
wants to merge 8 commits into
base: youlalala
Choose a base branch
from
Open

Step2 #721

Show file tree
Hide file tree
Changes from all 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
17 changes: 16 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,24 @@
## 온라인 코드 리뷰 과정
* [텍스트와 이미지로 살펴보는 온라인 코드 리뷰 과정](https://github.com/next-step/nextstep-docs/tree/master/codereview)

## 요구사항
## 기능 요구사항
### 1단계 - 레거시 코드 리팩토링
- deleteQuestion 에서 비즈니스 로직을 도메인 모델로 리팩토링
- [X] 로그인 사용자와 질문한 사람이 같은 경우 삭제 가능
- [X] 질문자와 답변글의 모든 답변자가 같은 경우 삭제 가능
- [X] 삭제 이력 남기기

### 2단계 - 수강신청(도메인 모델)
- [X] 과정(Course)은 기수 단위로 운영, 여러개의 강의(Session)존재
- [X] 강의는 시작일과 종료일이 존재
- [X] 강의는 강의 커버 이미지가 존재
- [X] 이미지 크기는 1MB 이하
- [X] 이미지 확장자는 gif, jpg, jpeg, png, svg 만 가능
- [X] width는 300픽셀, height는 200픽셀 이상이, width와 height의 비율은 3:2
- [X] 강의는 무료강의와 유료강의 분류
- [X] 무료강의 - 최대 수강 인원 제한 X
- [X] 유료강의 - 최대 수강 인원 제한 O
- [X] 유료강의 - 수강생이 결제한 금액과 수강료가 일치할 때 수강 신청이 가능
- [X] 강의 상태 : 준비중, 모집중, 종료
- [X] 강의 상태가 모집중일 때만 수강신청 가능
- [X] 결제 정보는 payments 모듈을 통해 관리, 결제 정보는 Payment 객체
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package nextstep.courses;

public class CannotEnrollSessionException extends RuntimeException {
public CannotEnrollSessionException(String message) {
super(message);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package nextstep.courses;

public class InvalidCoverImageException extends RuntimeException {

public InvalidCoverImageException(String message) {
super(message);
}
}
12 changes: 11 additions & 1 deletion src/main/java/nextstep/courses/domain/Course.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

public class Course {
private Long id;
private Long generation;

private String title;

Expand All @@ -13,17 +14,25 @@ public class Course {

private LocalDateTime updatedAt;

private Sessions sessions;

public Course() {
}

public Course(String title, Long creatorId) {
this(0L, title, creatorId, LocalDateTime.now(), null);
this(0L, 0L, title, creatorId, new Sessions(), LocalDateTime.now(), null);
}

public Course(Long id, String title, Long creatorId, LocalDateTime createdAt, LocalDateTime updatedAt) {
this(id, 0L, title, creatorId, new Sessions(), createdAt, updatedAt);
}

public Course(Long id, Long generation, String title, Long creatorId, Sessions sessions, LocalDateTime createdAt, LocalDateTime updatedAt) {
this.id = id;
this.generation = generation;
this.title = title;
this.creatorId = creatorId;
this.sessions = sessions;
this.createdAt = createdAt;
this.updatedAt = updatedAt;
}
Expand All @@ -50,4 +59,5 @@ public String toString() {
", updatedAt=" + updatedAt +
'}';
}

}
38 changes: 38 additions & 0 deletions src/main/java/nextstep/courses/domain/CoverImage.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package nextstep.courses.domain;

import nextstep.courses.InvalidCoverImageException;

public class CoverImage {

private final int width;
private final int height;
private final int sizeInBytes;

Choose a reason for hiding this comment

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

Suggested change
private final int sizeInBytes;
private final int sizeInBytes;

👀

private final ImageExtension extension;

public CoverImage(int width, int height, int sizeInBytes, ImageExtension extension) {
this.width = width;
this.height = height;
this.sizeInBytes = sizeInBytes;
this.extension = extension;
validate();
}

private void validate() {
if (sizeInBytes > 1_000_000) {
throw new InvalidCoverImageException("1MB를 초과하는 이미지입니다.");
}
if (width < 300) {
throw new InvalidCoverImageException("너비는 300px 이상이어야 합니다.");
}
if (height < 200) {
throw new InvalidCoverImageException("높이는 200px 이상이어야 합니다.");
}
if (width * 2 != height * 3) {
throw new InvalidCoverImageException("비율은 3:2여야 합니다.");
}
if (extension == null) {

Choose a reason for hiding this comment

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

null인 케이스도 있겠지만 확장자가 ImageExtension에 포함되지 않는 경우도 예외처리되면 좋겠네요!

throw new InvalidCoverImageException("지원하지 않는 확장자입니다.");
}
}

}
16 changes: 16 additions & 0 deletions src/main/java/nextstep/courses/domain/ImageExtension.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package nextstep.courses.domain;

public enum ImageExtension {
GIF("gif"),
JPG("jpg"),
JPEG("jpeg"),
PNG("png"),
SVG("svg");

private final String value;

ImageExtension(String value) {
this.value = value;
}

}
43 changes: 43 additions & 0 deletions src/main/java/nextstep/courses/domain/Session.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
package nextstep.courses.domain;

import nextstep.courses.CannotEnrollSessionException;
import nextstep.payments.domain.EnrollmentPolicy;
import nextstep.payments.domain.Payment;

import java.time.LocalDate;

public class Session {

private final LocalDate startDate;
private final LocalDate endDate;
private final CoverImage coverImage;
private final SessionStatus status;
private final EnrollmentPolicy enrollmentPolicy;

Choose a reason for hiding this comment

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

(반영하지 않으셔도 됩니다.) Session이라는 큰 추상적 표현을 생각해보면 Session을 인터페이스로 두고, "유료강의"와 "무료강의"라는 구현체를 두는 형태를 고민해볼 수 있을 것 같아요 😄


private int currentEnrolledCount = 0;

public Session(LocalDate startDate,
LocalDate endDate,
CoverImage coverImage,
SessionStatus status,
EnrollmentPolicy enrollmentPolicy) {
this.startDate = startDate;
this.endDate = endDate;
this.coverImage = coverImage;
this.status = status;
this.enrollmentPolicy = enrollmentPolicy;
}

public void enroll(Payment payment) {
if (!this.status.canEnroll()) {
throw new CannotEnrollSessionException("모집 중이 아닙니다.");
}

if (!enrollmentPolicy.canEnroll(currentEnrolledCount, payment)) {
throw new CannotEnrollSessionException("수강 조건이 맞지 않습니다.");
}

currentEnrolledCount++;

Choose a reason for hiding this comment

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

수강 수를 넣을 수도 있지만 누가 들었는지 수강생을 저장해보는건 어떨까요? NsUser를 활용해보면 좋을 것 같아요 :)

}

}
11 changes: 11 additions & 0 deletions src/main/java/nextstep/courses/domain/SessionStatus.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package nextstep.courses.domain;

public enum SessionStatus {
READY,
ENROLLING,
COMPLETED;

public boolean canEnroll() {
return this == ENROLLING;
}
}
16 changes: 16 additions & 0 deletions src/main/java/nextstep/courses/domain/Sessions.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package nextstep.courses.domain;

import java.util.List;

public class Sessions {

Choose a reason for hiding this comment

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

일급컬랙션 👍


private List<Session> sessions;

public Sessions() {
}
Comment on lines +7 to +10

Choose a reason for hiding this comment

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

Suggested change
private List<Session> sessions;
public Sessions() {
}
private final List<Session> sessions;
public Sessions() {
sessions = new ArrayList<>();
}

이렇게 보완이 되어야할 것 같네요 😄


public Sessions(List<Session> sessions) {
this.sessions = sessions;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import nextstep.courses.domain.Course;
import nextstep.courses.domain.CourseRepository;
import nextstep.courses.domain.Sessions;
import org.springframework.jdbc.core.JdbcOperations;
import org.springframework.jdbc.core.RowMapper;
import org.springframework.stereotype.Repository;
Expand Down
5 changes: 5 additions & 0 deletions src/main/java/nextstep/payments/domain/EnrollmentPolicy.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package nextstep.payments.domain;

public interface EnrollmentPolicy {

Choose a reason for hiding this comment

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

각 구현체마다 잘 추상화해주셨네요 👍

boolean canEnroll(int currentEnrolledCount, Payment payment);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package nextstep.payments.domain;

public class FreeEnrollmentPolicy implements EnrollmentPolicy {

@Override
public boolean canEnroll(int currentEnrolledCount, Payment payment) {
return payment.isSameAmount(0L);
}
Comment on lines +6 to +8

Choose a reason for hiding this comment

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

어떻게보면 무료 강의는 비교 구문 자체가 필요없지 않을까요? 👀

}
18 changes: 18 additions & 0 deletions src/main/java/nextstep/payments/domain/PaidEnrollmentPolicy.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package nextstep.payments.domain;

public class PaidEnrollmentPolicy implements EnrollmentPolicy {

private final int maxEnrolledCount;
private final int price;
Comment on lines +3 to +6

Choose a reason for hiding this comment

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

가격은 어떻게보면 Session이 들고 있어야하는 정보가 아닐까요?



public PaidEnrollmentPolicy(int maxEnrolledCount, int price) {
this.maxEnrolledCount = maxEnrolledCount;
this.price = price;
}

@Override
public boolean canEnroll(int currentEnrolledCount, Payment payment) {
return currentEnrolledCount < maxEnrolledCount && payment.isSameAmount((long) price);
}
}
4 changes: 4 additions & 0 deletions src/main/java/nextstep/payments/domain/Payment.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,8 @@ public Payment(String id, Long sessionId, Long nsUserId, Long amount) {
this.amount = amount;
this.createdAt = LocalDateTime.now();
}

public boolean isSameAmount(Long amount) {
return this.amount.equals(amount);
}
}
18 changes: 9 additions & 9 deletions src/main/java/nextstep/qna/domain/Answers.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import java.util.ArrayList;
import java.util.List;
import java.util.stream.Collectors;

public class Answers {
private final List<Answer> answers;
Expand All @@ -22,16 +23,15 @@ public List<Answer> getAnswers() {
}

public List<DeleteHistory> deleteAll(NsUser loginUser) {
for (Answer answer : answers) {
if (!answer.isOwner(loginUser)) {
throw new CannotDeleteException("다른 사람이 쓴 답변이 있어 삭제할 수 없습니다.");
}
if(!canAllDelete(loginUser)) {
throw new CannotDeleteException("다른 사람이 쓴 답변이 있어 삭제할 수 없습니다.");
}
return answers.stream().map(Answer::delete).collect(Collectors.toList());
}

private boolean canAllDelete(NsUser loginUser) {
return answers.stream().filter(answer -> answer.isOwner(loginUser)).count()
== answers.size();

List<DeleteHistory> deleteHistories = new ArrayList<>();
for (Answer answer : answers) {
deleteHistories.add(answer.delete());
}
return deleteHistories;
}
}
52 changes: 52 additions & 0 deletions src/test/java/nextstep/courses/domain/CoverImageTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
package nextstep.courses.domain;

import nextstep.courses.InvalidCoverImageException;
import org.junit.jupiter.api.Test;

import static org.assertj.core.api.AssertionsForClassTypes.assertThatCode;
import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;

public class CoverImageTest {

@Test
void 유효한_커버이미지는_예외없이_생성되고_검증_통과() {
assertThatCode(() ->
new CoverImage(600, 400, 500_000, ImageExtension.JPG)
).doesNotThrowAnyException();
}

@Test
void 크기가_1MB_초과하면_예외발생() {
assertThatThrownBy(() ->
new CoverImage(600, 400, 2_000_000, ImageExtension.PNG)
).isInstanceOf(InvalidCoverImageException.class);
}

@Test
void 너비가_300미만이면_예외발생() {
assertThatThrownBy(() ->
new CoverImage(299, 400, 500_000, ImageExtension.PNG)
).isInstanceOf(InvalidCoverImageException.class);
}

@Test
void 높이가_200미만이면_예외발생() {
assertThatThrownBy(() ->
new CoverImage(600, 199, 500_000, ImageExtension.PNG)
).isInstanceOf(InvalidCoverImageException.class);
}

@Test
void 비율이_3대2_아니면_예외발생() {
assertThatThrownBy(() ->
new CoverImage(600, 500, 500_000, ImageExtension.PNG)
).isInstanceOf(InvalidCoverImageException.class);
}

@Test
void 지원하지_않는_확장자면_예외발생() {
assertThatThrownBy(() ->
new CoverImage(600, 400, 500_000, null)
).isInstanceOf(InvalidCoverImageException.class);
}
}
42 changes: 42 additions & 0 deletions src/test/java/nextstep/courses/domain/SessionTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package nextstep.courses.domain;

import nextstep.courses.CannotEnrollSessionException;
import nextstep.payments.domain.PaidEnrollmentPolicy;
import org.junit.jupiter.api.Test;

import java.time.LocalDate;

import static nextstep.payments.PaymentTest.PAYMENT_1000;
import static org.assertj.core.api.AssertionsForClassTypes.assertThatCode;
import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;

public class SessionTest {

public static final Session ReadySession = new Session(
LocalDate.of(2025, 1, 1),
LocalDate.of(2025, 12, 31),
new CoverImage(600, 400, 500_000, ImageExtension.JPG),
SessionStatus.READY,
new PaidEnrollmentPolicy(100, 1000));

public static final Session EnrollingSession = new Session(
LocalDate.of(2025, 1, 1),
LocalDate.of(2025, 12, 31),
new CoverImage(600, 400, 500_000, ImageExtension.JPG),
SessionStatus.ENROLLING,
new PaidEnrollmentPolicy(100, 1000));

@Test
void 모집중이_아닐_때_수강신청_불가능() {
assertThatThrownBy(() ->
ReadySession.enroll(PAYMENT_1000)
).isInstanceOf(CannotEnrollSessionException.class);
}

@Test
void 모집중일_때_수강신청_가능() {
assertThatCode(() -> EnrollingSession.enroll(PAYMENT_1000))
.doesNotThrowAnyException();

}
}
Loading