Skip to content

Conversation

@chanani
Copy link

@chanani chanani commented Dec 10, 2025

안녕하세요 ! 수강신청 4단계 리뷰 요청드립니다.🙇‍♂️

최초 테이블 설계 시 session 테이블에서 cover_image_id를 가지고 있었습니다.
1:1 관계에서 1:N 상태로 변경되면서 기존과 다르게 cover_image 테이블에서 session_id를 가지고 있어야 된다고 판단되어 불가피하게 테이블을 수정했습니다.
DB 테이블에 데이터가 존재한다는 가정하에 리팩터링을 진행하라는 요구사항이 있었는데, 이를 지키지 못했습니다.

제가 마지막 요구사항을 잘 이해하고 진행한건지 확신이 들지 않습니다.
우아한테크코스(무료), 우아한테크캠프 Pro(유료)와 같이 선발된 인원만 수강 가능해야 한다. 라는 조건에서 선발 여부도 수정이 가능한 쪽으로 작업을 진행했는데, 혹시라도 요구사항을 제대로 파악하지 못한 부분이 있을 경우 수정하겠습니다.

… 후 필요한 클래스에 상속(SessionCore 요구사항 추가로 인해 상속이 아닌 인스턴스 변수로 사용하는 것이 좋겠다고 판단.)
SessionCoreV2에서 추가된 요구사항 적용 예정
- session 테이블 에서 cover_image_id를 관리했는데 cover_image 테이블에서 session_id 관리할 수 있도록 수정
- DB 테이블에 데이터가 존재한다는 가정하에 리팩터링 진행하라는 요구사항이 있었는데, 최초 DB 설계 시 문제로 이를 지키지 못했습니다. 하지만 실무에서 이런 상황이 온다면, 기존 데이터에 문제되지 않도록 스크립트를 작성할 것같습니다.
refactor : 수강신청 객체에 상태값 추가 및 테스트 코드 작성
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.

드디어 미션의 마지막 단계넨요. 👍
요구사항은 정확히 잘 파악해 구현 잘 했네요.
단, Session의 복잡도를 낮추고 인터페이스 사용과 관련한 피드백 남겼으니 한번 고민해 보는 시간을 가지면 좋겠습니다.

import java.util.ArrayList;
import java.util.List;

public class CoverImages {
Copy link
Contributor

Choose a reason for hiding this comment

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

일급 콜렉션 적용 👍


import java.util.Arrays;

public enum EnrollmentStatus {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍


import java.util.Arrays;

public enum SelectionStatus {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@@ -0,0 +1,15 @@
package nextstep.courses.domain.session.constant;

public enum SessionRecruitmentStatus {
Copy link
Contributor

Choose a reason for hiding this comment

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

상태 값을 분리 👍

}

private boolean isSelectedAndCancelled(EnrollmentStatus enrollmentStatus) {
return this.selectionStatus.equals(SelectionStatus.SELECTED) && enrollmentStatus.equals(EnrollmentStatus.CANCELLED);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return this.selectionStatus.equals(SelectionStatus.SELECTED) && enrollmentStatus.equals(EnrollmentStatus.CANCELLED);
return this.selectionStatus.isSelected() && enrollmentStatus.isCancled();

enum도 class와 같이 메시지를 보내는 방식으로 구현 가능함


import java.time.LocalDateTime;

public interface SessionCoreFacade {
Copy link
Contributor

Choose a reason for hiding this comment

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

인터페이스의 메서드가 너무 많다.
인터페이스의 메서드가 너무 많을 경우 인터페이스 설계가 잘못되었을 가능성이 높음
이럴 경우 인터페이스를 책임에 따라 분리하거나, 불필요한 메서드를 제거할 필요가 있음
또한 이 인터페이스가 필요한지에 대해서도 고민해 보면 좋겠다.

Comment on lines 71 to 78
SessionRecord sessionRecord = sessionRepository.findById(enrollment.getSessionId());
List<EnrollmentRecord> enrollmentRecords = enrollmentRepository.findBySessionId(enrollment.getSessionId());
Session session = sessionRecord.toSession(null, null, toEnrollments(enrollmentRecords));
Session session = SessionMapper.toDomain(sessionRecord, null, null, toEnrollments(enrollmentRecords));
session.addEnrollment(enrollment, payment);

paymentRepository.save(payment);

return enrollmentRepository.save(enrollment);
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을 통해 수강신청에 필요한 객체를 아래의 Enrollment와 같은 객체로 생성한 후 위임함으로써 복잡도를 낮추는 방식으로 접근해 보는 것은 어떨까?

아래 코드에서 Enrollment는 수강 신청 가능 여부를 판단하는 로직을 담당하는 객체로 찬한님 코드의 Enrollment와는 다름
찬한님의 Enrollment는 아래 코드에서 Student임

    SessionRecord sessionRecord = ...
    List<EnrollmentRecord> enrollmentRecords = ...
    Enrollment enrollment = sessionRecord.createEnrollment(enrollmentRecords);
    Student student = enrollment.enroll(user);
    enrollmentRepository.save(student.getUserId(), student.getSessionId());

위 코드에서 리팩터링 힌트를 얻을 수 있는 부분이 있다면 리팩터링해 본다.

@chanani
Copy link
Author

chanani commented Dec 14, 2025

안녕하세요 !
수강신청 프로세스를 EnrolmentApply 클래스로 위임하여 진행해봤습니다.

모든 작업을 컴파일에러가 발생하지 않도록 진행하려고 노력했고, 학습목표에 나와있는 것처럼 최초 신규 기능 추가 시 스트랭글러 패턴을 적용하려고 interface를 통해서 리팩터링을 시도해봤습니다. 하지만 제가 잘못 이해하고 부분별하게 사용하여 인터페이스의 메서드가 많아졌던 것 같습니다 !🙇‍♂️

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.

지금까지 수강 신청 미션 진행하느라 수고했어요. 👍
도메인 객체 설계 후 DB 매핑하는 방식으로 진행했는데 어떠셨나요?
이번 미션을 통해 도메인 객체 설계와 테이블 설계가 어떻게 다른지를 경험하는 계기가 됐으면 좋겠습니다.
또한 현장의 레거시 코드에 대한 점진적인 리팩터링도 도전해 보면 좋겠습니다.
이번 미션 여기서 마무리할께요.
사다리타기 미션은 선택인 만큼 찬한님 여력에 따라 진행여부 결정하면 됩니다.

@javajigi javajigi merged commit 21170bc into next-step:chanani Dec 14, 2025
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