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 - 수강신청(도메인 모델) #393

Open
wants to merge 13 commits into
base: parkje0927
Choose a base branch
from

Conversation

parkje0927
Copy link

미션 수행 내용

  • 수강신청 구현 내용은 아래와 같습니다.
- 수강신청을 진행한다는 가정 하에 로직을 구현했고, 도메인 중심적으로 설계 했습니다.
- 현재 오픈된 기수에 맞는 강의 목록을 가져와서 보여주는 로직이 있다고 가정했습니다.
- 그리고 그 목록 중에서 유저가 특정 강의를 선택했다고 할 때 강의에 대한 세부 정보를 보여주는 로직이 필요하다고 생각했습니다.
- 마지막으로 수강 신청을 진행하는 flow 를 고려하여 설계했습니다.

확인 부탁드립니다.

Copy link
Contributor

@javajigi javajigi left a comment

Choose a reason for hiding this comment

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

2단계 미션 구현하느라 수고했어요. 👍
2단계임에도 불구하고 벌써 db까지 고려해 구현했네요.
그렇다보니 기존 구현 방식에 얽매이는 느낌입니다.
2단계는 db는 고려하지 않고 도메인 객체 설계와 구현에만 집중해 보는 것은 어떨까요?

this(0L, title, creatorId, 1, null, LocalDateTime.now(), null);
}

public Course(Long id, String title, Long creatorId, int cardinalNumber, Sessions sessions,
Copy link
Contributor

Choose a reason for hiding this comment

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

주생성자를 생성자에 마지막에 구현하는 것이 관례

public List<Session> getPossibleSessionList() {
return this.sessions.getPossibleSessionList();
}

Copy link
Contributor

Choose a reason for hiding this comment

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

새로운 Session을 추가하는 addSession(Session session)과 같은 메서드도 추가하는 것은 어떨까?

Comment on lines +12 to +16
private int size;

private int width;

private int height;
Copy link
Contributor

Choose a reason for hiding this comment

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

원시값 포장, 관련 있는 필드 값을 객체로 분리하는 차원에서 ImageFileSize, ImageSize와 같은 객체를 분리하는 것은 의미가 있을까?


ImageType() {

}
Copy link
Contributor

Choose a reason for hiding this comment

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

이미지 파일이 유효한 파일인지에 대한 체크 로직을 이 객체에 구현하는 것은 어떨까?

public class FreeSessionType implements SessionType {

private Long amount;
private int maxCapacity;
Copy link
Contributor

Choose a reason for hiding this comment

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

amount, maxCapacity 인스턴스 변수로 초기화하고 아무 곳에서도 사용하지 않고 있다.
굳이 인스턴스로 구현할 필요가 있을까?

Comment on lines +15 to +17
private LocalDate startDt;

private LocalDate endDt;
Copy link
Contributor

Choose a reason for hiding this comment

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

이 두 개의 값은 서로 연관되어 있는 값이다.
이 두 개의 값을 가지는 객체를 분리하는 것은 어떨까?
분리한 후 시작일은 종료일보다 빨라야 한다와 같은 유효성 체크 로직을 추가하는 것은 어떨까?

private LocalDate startDt;

private LocalDate endDt;

Copy link
Contributor

Choose a reason for hiding this comment

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

Session별로 강의 커버 이미지 정보도 가지고 있어야 하지 않을까?

Comment on lines +21 to +23
sessionA = new Session(1L, "test1", SessionStatus.RECRUITING, new FreeSessionType(), LocalDate.now(), LocalDate.now());
sessionB = new Session(2L, "test2", SessionStatus.TERMINATION, new FreeSessionType(), LocalDate.now(), LocalDate.now());
sessionC = new Session(3L, "test3", SessionStatus.RECRUITING, new FreeSessionType(), LocalDate.now(), LocalDate.now());
Copy link
Contributor

Choose a reason for hiding this comment

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

Session 객체와 같이 인스턴스 변수가 많은 객체를 테스트하려면 객체를 생성하는데 어려움이 있다.
중복 코드 또한 많이 발생해 Session을 생성할 때 생성자의 인자가 변경되는 경우 변경할 부분이 많아진다.
https://www.arhohuttunen.com/test-data-builders/ 문서 참고해 Session과 같이 복잡한 객체의 테스트 데이터를 생성할 때 어떤 방법을 사용할 것인지 선택해 보면 좋겠다.
이번 기회에 내가 선호하는 방법을 적용해 보고 앞으로도 쭈욱 활용하는 방식이면 좋겠다.

void 유료_강의_수강_신청_실패_수강_인원_초과_테스트() {
assertThatIllegalArgumentException()
.isThrownBy(() -> paidSession.isPossibleToRegister(40_000L, 30));
}
Copy link
Contributor

Choose a reason for hiding this comment

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

session.enroll(nsUser);와 같은 메시지를 보내면 session이 수강 신청 가능 여부룰 확인하고, 수강 신청이 가능하면 수강생 목록에 추가하는 방식으로 구현해 보면 어떨까?

2단계임에도 불구하고 벌써 db까지 고려하다보니 도메인 로직 구현에 집중하는데 어려움이 있는 느낌이다.
db를 고려하지 않고, 앞의 로또, 사다리와 같은 관점에서 도메인 객체 설계와 로직 구현에 집중하면 어떨까?

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