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

Step2 - 도메인설계 #486

Open
wants to merge 33 commits into
base: boy0516
Choose a base branch
from
Open

Step2 - 도메인설계 #486

wants to merge 33 commits into from

Conversation

boy0516
Copy link

@boy0516 boy0516 commented Apr 14, 2024

안녕하세요 2단계 리뷰 부탁드립니다.

과정이 기수별로 관리된다고했는데 기수를 과정의 상위로 봐야할지 과정의 일부로 봐야할지 잘 모르겠습니다.
어떤 방향으로 설정하는게 맞을까요?

@boy0516 boy0516 changed the base branch from master to boy0516 April 14, 2024 09:27
Copy link

@ksy90101 ksy90101 left a comment

Choose a reason for hiding this comment

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

안녕하세요~
2단계도 고생 많으셨습니다.
간단한 피드백을 남겼으니 참고해주세요!
질문이 있다면 언제든 DM주세요~

cf) 좀더 요구사항 파악을 해보시면 좋을거 같습니다!!

@@ -9,13 +9,12 @@ public class Course {

private Long creatorId;

private Sessions sessions;

Choose a reason for hiding this comment

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

과정이 기수별로 있다. 라는것은 과정 == Course // 기수 == Session으로

에를들어 현재 듣고 계신 TDD 과정이 있고 1기~18기까지가 있었죠.

즉, 1개의 Course에 여러개의 Session이 잇어야 할거 같습니다.

Comment on lines +3 to +8
public class FreeSession {
private final Session session;

public FreeSession(Session session) {
this.session = session;
}

Choose a reason for hiding this comment

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

이걸 따로 만든 이유가 있나요?
유료와 무료를 나눠서 객체를 관리하고 싶다면 조합보단 상속이 더 어울리는 형태가 아닐까 생각이 들어요.


public class ImageRate {

private final int MIN_WIDTH_SIZE = 300;

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 MIN_WIDTH_SIZE = 300;
private static final int MIN_WIDTH_SIZE = 300;

}

private void validRate(int width, int height) {
if (width / height != 3 / 2) {

Choose a reason for hiding this comment

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

3/2도 상수화를 해두면 어떨까요?


private final int MAX_SIZE = 1;

private int size;

Choose a reason for hiding this comment

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

이 size의 용량을 MB로 설정하셨군요.
기본적으로 어떤 용량으로 설정하는지 한번 고민해보면 좋을거 같습니다.
예를들어 S3는 용량을 default가 무엇일까요?
혹은 서버에 저장할때.

public class PaySession {

private final Session session;
private int joinLimit;

Choose a reason for hiding this comment

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

      - [] 유료 강의 최대 수강 인원 초과 검증.

이게 아직 안된거 같아요!

private final LocalDateTime endDate;

public Period() {
this(LocalDateTime.now(), LocalDateTime.now().plusWeeks(1));

Choose a reason for hiding this comment

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

무조건 기간은 일주일인가요?

}

private void validPaid(Payment payment) {
if(!payment.isPaid())

Choose a reason for hiding this comment

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

실제 강의엔 수강료가 있고 그 수강료 만큼 가격을 내야 하지 않을까요?

유료 강의는 수강생이 결제한 금액과 수강료가 일치할 때 수강 신청이 가능하다.

this.period = new Period();
}

public void join(JoinUser joinUser) {

Choose a reason for hiding this comment

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

강의 수강신청은 강의 상태가 모집중일 때만 가능하다.

이것도 필요하지 않을까요?

@@ -0,0 +1,9 @@
package nextstep.courses.domain;

public class FreeSession {

Choose a reason for hiding this comment

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

무료강의는 어떻게 신청을 하나요?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants