Skip to content

Conversation

ke-62
Copy link

@ke-62 ke-62 commented Oct 2, 2025

안녕하세요, 준수님. 아직 직접 뵌 적은 없지만 리뷰어, 리뷰이로서 처음 인사드리게 되었네요.
아직 배우는 단계라 많이 부족하지만 잘 부탁드립니다 🙇‍♀️

저의 코드를 처음 보시는 것이다 보니 제 코드에 대해 간단히 설명드리면,

**view (InputView, OutputView, ResultView)**는 사용자 입출력과 결과를 출력하는 역할을 수행합니다.
사용자의 입력이나 변수에 따라 값이 달라지는 것은 ResultView, 항상 일정한 멘트만을 출력하는 것은 OutputView로 분리하였습니다.

LottoController.java는 Controller로 view와 model을 연결하는 역할을 합니다.
domain/LottoNumber.java는 개별 로또 번호(1~45)의 유효성을 검사하는 값 객체입니다.
domain/Lotto.java는 6개의 LottoNumber를 래핑하여 하나의 로또 티켓을 표현합니다.
domain/createList.java는 로또 번호의 범위 상수를 정의하고 자동 로또 번호 생성을 담당합니다.
domain/LottoTickets.java는 수동/자동으로 구매한 모든 로또를 관리하는 로또 목록 역할을 합니다.
domain/LottoService.java는 로또 번호 파싱, 보너스 볼 유효성 검사, 당첨 통계 계산을 수행하는 역할을 합니다.
domain/MatchCount.java는 로또들의 등수별 당첨 개수를 계산하고 저장합니다.
domain/LottoPrice.java는 등수별 당첨 금액을 정의하는 enum입니다.
domain/ProfitRate.java는 최종 당첨 금액과 구매 금액을 기반으로 총 수익률을 계산합니다.
domain/Money.java는 구매 금액을 캡슐화하고 로또 구매 가능 개수를 계산합니다.
LottoTicketCount.java 구매 금액으로 살 수 있는 로또 티켓의 개수를 나타냅니다.
Match.java 두 로또 사이의 일치하는 번호 개수를 계산하는 로직을 가집니다.
LottoTotalPrice.java MatchCount를 바탕으로 총 당첨 금액 합계를 계산합니다.

이번 미션을 하면서 가장 고민이 되었던 부분은 예외가 발생하였을 때 (수동으로 6개의 숫자를 입력 받을 때 그 6개의 숫자중 중복된 것이 존재한다, 보너스 볼이 당첨번호와 일치한다) 이 예외처리를 어디에서 할 지 였습니다. 처음 1-2단계에서 했던 것 처럼 관련 클래스에 예외처리 로직을 추가해 예외를 처리했습니다. 문제는 이렇게 하다 보니 테스트 코드를 작성할 때 “이 예외 처리가 어디에 있었지?” 하고 헷갈릴 때가 있었습니다. 그래서 차라리 예외 처리만 전담하는 클래스로 분리하는 것이 더 낫지 않을까 고민하게 되었습니다. 예외 처리 전용 클래스를 따로 두는 것이 일반적인 접근인지 궁금합니다 🤔

또 궁금한 점은 제 코드가 다른 분들에 비해 클래스 수가 많은 것 같다는 점입니다. 단일 책임 원칙을 의식하다 보니 규모가 조금만 커져도 “어느 클래스에 뭐가 있었지?” 하며 헷갈리고 불편하더라고요. 혹시 클래스 분리를 할 때 어떤 기준을 두고 계신지, 실제로는 어떻게 관리하시는지 궁금합니다!!

마지막으로 저는 Controller에서는 View와 Model을 연결하는 역할만 수행하도록 하고 싶어 반복문 사용을 최대한 지양했습니다. 하지만 수동 입력으로 로또 번호를 받는 과정에서, 입력 개수가 매번 달라지다 보니 어쩔 수 없이 Controller 내부에 for문을 선언하게 되었습니다. 그렇지 않으면 Domain에서 InputView를 import해야 하는 상황이 발생하더군요. 혹시 Controller에서는 단순히 호출만 담당하면서도 MVC 패턴을 잘 지킬 수 있는 구조로 코드를 작성할 수 있을까요?

항상 코드 리뷰 해주셔서 감사합니다 !!😊

아, 그리고 제가 아직 커밋에 익숙하지 않아서 형식을 통일하려고 노력하고는 있지만, , 중간중간 깜빡해서 커밋들이 제각각인데.. 흐린눈 하고 봐주시면 감사하겠습니다.😅

@ke-62 ke-62 changed the base branch from main to ke-62 October 4, 2025 08:39
Copy link

@gogo1414 gogo1414 left a comment

Choose a reason for hiding this comment

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

안녕하세요, 고은님!
로또 미션 3, 4, 5단계 리뷰를 맡은 김준수라고 합니다 😊

코드에 대한 설명과 요구사항 모두 잘 정리해주셔서 감사해요. 5단계 요구사항이 빠진 것 같은데, 해당 부분 확인해서 추가해주시면 좋을 것 같네요!

몇가지 코멘트와 수정을 요청 드리려고 해요! 모든 내용을 반영하기 어려우시다면 최대한 가능한 부분만 반영해주셔도 좋습니다!


수정 요구 사항

1. 코드 포맷팅을 신경써주세요!

  • 현재 줄바꿈이 적용된 부분과 적용되지 않은 부분 등 각 클래스별로 차이가 존재하는데, 하나로 통일 해주시면 좋을 것 같습니다.
  • 여러 개의 인자를 주입하는 코드를 작성할 때와 메서드 체이닝을 활용할 때 가독성을 위해 줄바꿈을 적용해보는건 어떨까요?

2. 클래스 네이밍을 통일해주세요!

  • LottoTicketsList<Lotto>를 내부에 가지고 있는 형태인데, Lotto, LottoTicket 차이점이 있을까요? 하나로 통일해서 사용하면, 더 쉽게 이해가 가능할 것 같아요!

3. 로또 게임의 동작 흐름을 한번 글로 정리해보면 어떨까요?

  • 로또 게임 시 돈을 1000원 미만으로 입력했을 경우 어떤 동작으로 진행되는게 좋을까요? 실제 저희가 로또를 구매한다고 했을 때 잔액이 부족하면 어떻게 될까요?
스크린샷 2025-10-04 오후 3 54 53
  • 입력 형태가 잘못되는 경우 예외를 발생시켜 게임을 중지하는 것과 다시 입력받게 하는 것 중 사용자는 어떤걸 더 편하게 생각할까요?
스크린샷 2025-10-04 오후 3 55 38
  • 구매 금액이 매우 많은 경우 어떻게 처리하면 좋을까요?
스크린샷 2025-10-04 오후 4 14 28

예외 처리 전용 클래스를 따로 두는게 좋을까요?

  • 좋은 질문이네요!
  • "이 규칙이 없다면 Lotto 객체가 존재할 수 있는지?" 와 같은 핵심 불변 조건은 접근법 A처럼 생성자에서 처리하기도 하고 여러 도메인에 걸친 비즈니스 규칙은 Validator와 같이 별도의 클래스로 분리하는 것을 고려할 수 있을 것 같아요. 두 가지 방법을 적절히 조합하며 이 책임은 누가 갖는 것이 가장 적절한지 계속 고민해보시고 결정하시면 될 것 같아요!

클래스 분리를 할 때 어떤 기준을 두고 계신지? 실제로 어떤 관리하시는지?

  • 단일 책임 원칙을 깊게 고민하신 점이 좋네요. 다만, 너무 작은 단위로 분리되어 전체 흐름을 파악하기 조금 어려운 점이 있었어요. 클래스 분리의 기준은 함께 변경될 가능성이 높은 데이터와 로직을 묶는 것(응집도)을 핵심으로 생각해보시면 좋습니다. 어떤 기능 변경이 필요할 때 여러 클래스를 수정하기보다, 가급적 하나의 클래스 안에서 변경이 완결되도록 만드는 거죠. 이 관점에서 현재 프로젝트를 다시 한번 살펴보시면 좋을 것 같습니다!

Controller에서는 단순히 호출만 담당하면서도 MVC 패턴을 잘 지킬 수 있는 구조로 코드를 작성할 수 있을까요?

  • 컨트롤러의 주요 역할은 애플리케이션의 작업 흐름을 제어하는 것입니다. 이 역할에 따라 컨트롤러는 View에 무엇을 요청하고 Service에 무엇을 전달할지 순서와 과정을 결정해요. 따라서 여러 입력을 받기 위해 for문을 사용하는 것은 흐름을 제어하는 컨트롤러의 자연스러운 역할이라고 생각해요.
  • 중요한 점은 컨트롤러가 이 흐름을 제어할 뿐, 비즈니스 로직을 직접 처리하지 않는다는 것만 기억해주시면 될 것 같아요. 예를 들어, 로또 번호를 검증하거나 로또 객체를 생성하는 것은 컨트롤러의 책임이 아니기 때문에, 컨트롤러는 View로부터 입력받은 데이터를 Service에 전달하여 처리를 요청해야 해요.

학교와 병행하며 미션을 진행하느라 어려움이 있으셨을 텐데 많은 고민을 하시면서 코드를 작성하신게 느껴져 좋았습니다👍👍 많은 내용을 반영하지 않으셔도 되니, 가능한 만큼 진행해주시면 좋겠네요! 추석 연휴 잘보내세요!!

Copy link

Choose a reason for hiding this comment

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

5단계 요구사항이 없어 보이는데 확인 부탁드려요!

Copy link
Author

Choose a reason for hiding this comment

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

5단계는 리팩토링이라 크게 적은게 없긴 한데, 추가해 두겠습니다!!

Comment on lines 14 to 32
class MatchCountTest {
@ParameterizedTest
@CsvSource({"1,2,3,4,5,6,1,2,3,4,5,6,7,false,MATCH_6", "1,2,3,4,5,7,1,2,3,4,5,6,7,true,MATCH_5_BONUS", "1,2,3,4,5,8,1,2,3,4,5,6,7,false,MATCH_5", "1,2,3,4,8,9,1,2,3,4,5,6,7,false,MATCH_4", "1,2,3,8,9,10,1,2,3,4,5,6,7,false,MATCH_3"})
@DisplayName("당첨 통계 테스트-보너스볼 포함")
void testMatchWithBonusBall(int n1, int n2, int n3, int n4, int n5, int n6, int w1, int w2, int w3, int w4, int w5, int w6, int bonus, boolean expectedHasBonus, LottoPrice expectedRank) { // boolean 파라미터 추가
Lotto lotto = new Lotto(new java.util.TreeSet<>(Arrays.asList(new LottoNumber(n1), new LottoNumber(n2), new LottoNumber(n3), new LottoNumber(n4), new LottoNumber(n5), new LottoNumber(n6))));


Lotto winningLotto = new Lotto(new java.util.TreeSet<>(Arrays.asList(new LottoNumber(w1), new LottoNumber(w2), new LottoNumber(w3), new LottoNumber(w4), new LottoNumber(w5), new LottoNumber(w6))));

LottoNumber bonusBall = new LottoNumber(bonus);

int matchCount = Match.getMatchCount(lotto, winningLotto);
boolean hasBonusBall = (lotto.contains(bonusBall) == 1);

LottoPrice actualRank = determineLottoPrice(matchCount, hasBonusBall);
assertEquals(expectedRank, actualRank);
}
}
Copy link

Choose a reason for hiding this comment

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

해당 테스트가 어떤 테스트인지 파악이 안되네요... 어떤 테스트일까요?
그리고 CsvSource에 여러 값들이 들어가 있는데, 어떤 의미인지도 모르겠어요.. 타인이 보기에 보다 더 쉽게 이해할 수 있게 만들려면 어떻게 하는게 좋을까요? 한번 고민해보시고 테스트 코드 수정해보시면 좋을 것 같아요!

Copy link
Author

Choose a reason for hiding this comment

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

이건w들은 정답 코드, n들은 비교 객체로 두고 당첨 등수가 올바르게 추가되는지 확인하기 위해 넣어둔 코드인데..
다시 보니까 너무 변수도 많고 번잡하네요.. 이 부분 다시 한번 수정해 보겠습니다ㅜㅜ!!

void manualLottoDuplicate() {
SortedSet<LottoNumber> duplicateNumbers = new TreeSet<>(Arrays.asList(new LottoNumber(1), new LottoNumber(2), new LottoNumber(3), new LottoNumber(4), new LottoNumber(5), new LottoNumber(5)));
assertThatThrownBy(() -> new Lotto(duplicateNumbers)).isInstanceOf(IllegalArgumentException.class).hasMessageContaining("로또 숫자는 6개여야 하며, 중복될 수 없습니다.");

Copy link

Choose a reason for hiding this comment

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

위의 테스트 코드들과 일관된 형태를 유지할 수 있게 줄바꿈을 제거해주세요!

Copy link
Author

Choose a reason for hiding this comment

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

넵 알겠습니다 🫡


public LottoTickets buyLotto(Money money) {

LottoTicketCount ticketNumber = Money.getTicketCount(money);
Copy link

Choose a reason for hiding this comment

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

Money money 인자를 전달 받을 때 내부에 금액(money)이 이미 정의 되어있을텐데, Money객체 내부에 있는 getTicketCount() 메서드에 Money 클래스를 전달하는 이유가 있을까요??

Copy link
Author

Choose a reason for hiding this comment

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

처음에는 금액으로부터 티켓 개수를 계산하는 기능을 Money의 책임으로 두고 싶었습니다. 다만 getTicketCount를 static으로 작성하다 보니 내부 필드 money에 직접 접근할 수 없어, 어쩔 수 없이 Money 객체를 인자로 전달받아 사용하게 되었습니다. 리뷰를 받고 생각해 보니 인스턴스 메서드로 두고 money.getTicketCount() 형태로 호출하는 편이 더 자연스러운 설계인 것 같습니다!

Comment on lines 21 to 23
public SortedSet<LottoNumber> getNumbers() {
return new TreeSet<>(numbers);
}
Copy link

Choose a reason for hiding this comment

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

Treeset를 새롭게 만들어 반환하는 이유가 있을까요?
내부에 존재하는 numbersSortedSet을 사용중이고 반환하는 타입도 SortedSet으로 지정되어 있는데, TreeSet으로 새롭게 만들어 반환하는 이유가 궁금하네요!

Copy link
Author

Choose a reason for hiding this comment

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

방어적 복사를 하고 싶은 의도가 가장 컸습니다. 처음에는 반환 타입이 SortedSet이니 그대로 생성할 수 있을 거라 생각했는데, SortedSet은 인터페이스라 직접 인스턴스를 만들 수 없어서 정렬과 중복 제거 특성을 가진 대표 구현체인 TreeSet으로 복사하여 반환하도록 했습니다.

Copy link

Choose a reason for hiding this comment

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

List 자료구조도 존재하고 해당 자료구조에도 정렬할 수 있는 로직이 존재하는데, 타입을 Set으로 선택하신 이유도 있을까요?

Lotto 객체 생성 특성상 한번 생성된 이후로 번호가 변경되거나 새로 삽입되지 않아서 이유가 궁금하네요!

Copy link
Author

Choose a reason for hiding this comment

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

처음에는 List를 사용해 코드를 작성했지만, 중복 체크와 정렬을 직접 처리해야 하는 불편함이 있었습니다. 1, 2단계 리뷰어님께서 “적절한 자료구조를 사용하면 코드가 훨씬 간결해진다”는 조언을 주셔서 여러 자료구조를 공부한 끝에 Set을 선택했습니다.
Set을 사용한 이유는 로또 번호의 특성상 중복이 허용되지 않고, 생성 이후 값이 변경되거나 추가되지 않는데, Set 자료구조는 중복을 자동으로 방지해 주고, 또한 TreeSet을 사용하면 번호가 항상 정렬된 상태로 유지되어 별도의 정렬 로직이 필요하지 않아 더욱 간결하게 코드를 짤 수 있기 떄문입니다.

Copy link

Choose a reason for hiding this comment

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

리스트 만드는 객체가 따로 필요하다고 생각하신 이유가 궁금하네요!

Copy link
Author

Choose a reason for hiding this comment

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

번호 생성 로직과 Lotto 객체의 역할을 분리하고 싶었습니다. Lotto는 단순히 번호를 담는 도메인 역할만 맡고, 실제 번호 생성은 별도의 클래스에서 담당하도록 두면 책임이 명확해지고 재사용성도 좋아진다고 생각했습니다. 다만 현재 구조처럼 정적 메서드 하나만 두는 경우라면 굳이 나눌 필요는 없을 것 같습니다..!

int match3Count = matchCount.getCount(LottoPrice.MATCH_3);
int match4Count = matchCount.getCount(LottoPrice.MATCH_4);
int match5Count = matchCount.getCount(LottoPrice.MATCH_5);
int matchSecond = matchCount.getCount(LottoPrice.MATCH_5_BONUS);
Copy link

Choose a reason for hiding this comment

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

MATCH_5_BONUS와 matchSecond 두 네이밍에서 어떤 연과성이 있어 해당 네이밍으로 작성하신건지 궁금하네요!

Copy link
Author

Choose a reason for hiding this comment

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

문제를 보면, 5개 숫자 일치에 보너스볼 까지 일치하면 2등 당첨이라고 적혀져 있어서 2등이라는 의미에서 Second라고 적었습니다. 그리고 second만 보고서는 어떨때 second인지 알기 어려울 것 같아서 MATCH_5_BONUS로 작성했습니다!

Copy link

@gogo1414 gogo1414 Oct 6, 2025

Choose a reason for hiding this comment

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

설명해주셔서 감사해요!
다만, matchSecond라는 네이밍을 처음 봤을 때 저는 2개만 맞은 걸로 파악이 되었어요..😅

형식을 유지해서 match5CountWithBonus라는 형태 혹은 네이밍 전체를 바꿔 1등부터 5등을 표현하는 네이밍으로 변경하면 이해하기 더 편할 것 같다는 생각이 드네요!

Copy link
Author

Choose a reason for hiding this comment

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

아 그렇수도 있겠네요..!
보너스볼 로직이 2단계..?3단게..? 에 추가된 거라 통일성 없게 제가 적어둔 것 같습니다
네이밍 전체를 1등-5등 으로 적는 게 더 좋을 것 같습니다!!

assertThrows(IllegalArgumentException.class, () -> new Lotto(duplicated));
@DisplayName("로또 번호 수동 입력 시 중복된 숫자가 들어올 경우 예외가 발생한다.")
void manualLottoDuplicate() {
SortedSet<LottoNumber> duplicateNumbers = new TreeSet<>(Arrays.asList(new LottoNumber(1), new LottoNumber(2), new LottoNumber(3), new LottoNumber(4), new LottoNumber(5), new LottoNumber(5)));
Copy link

Choose a reason for hiding this comment

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

매우 많은 수의 인자를 받는 경우 한 줄에 모든 인자를 적기보다 아래와 같이 줄바꿈을 통해 파악하기 쉬운 형태로 작성해보면 어떨까요?
아래와 같은 포맷팅은 많은 인자를 받을 때 뿐만 아니라 Stream을 활용한 메서드 체이닝을 활용할 때도 한 줄에 모두 작성했을 때와 줄바꿈을 통해 여러 줄에 걸쳐 작성을 했을 때 가독성이 더 좋은 코드를 작성할 수 있어요! (참고 하셔서 다른 줄도 같이 수정 진행해주시면 좋겠네요!)

Suggested change
SortedSet<LottoNumber> duplicateNumbers = new TreeSet<>(Arrays.asList(new LottoNumber(1), new LottoNumber(2), new LottoNumber(3), new LottoNumber(4), new LottoNumber(5), new LottoNumber(5)));
SortedSet<LottoNumber> duplicateNumbers = new TreeSet<>(
Arrays.asList(
new LottoNumber(1),
new LottoNumber(2),
new LottoNumber(3),
new LottoNumber(4),
new LottoNumber(5),
new LottoNumber(5)
)
);

Copy link
Author

Choose a reason for hiding this comment

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

네 조언 감사합니다!! 수정하겠습니당😆

Copy link
Author

Choose a reason for hiding this comment

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

이 부분 인텔리제이에서 자동 정렬 하는 게 있더라구요..!
always는 너무 사소한 단위까지 잘라 조금 번잡해 보이는 것 같아서 if long 일때로 해 두었습니다
image

Copy link

@gogo1414 gogo1414 Oct 6, 2025

Choose a reason for hiding this comment

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

지금은 인텔리제이의 자동정렬로 충분할 수 있지만, 협업 혹은 직무에 나가서는 특정 코드 컨벤션을 정하고 해당 컨벤션에 맞게 작업을 하실 수도 있어요!

만약, 시간이 되신다면 코드 컨벤션에 관한 내용을 함께 찾아보면 많은 도움이 되실 것 같습니다 😄

@ke-62
Copy link
Author

ke-62 commented Oct 6, 2025

1차 수정 완료했습니다 !!
이번에는 단일 책임 원칙을 위반하지 않으면서, 합칠 수 있는 부분을 최대한 통합해 클래스 수를 줄이는 것에 집중했습니다.
아직 클래스가 많은 편이긴 하지만 😅, 수정 전보다 훨씬 구조가 명확해지고 파악하기 쉬워졌다고 생각합니다.

수정하면서 한 가지 궁금한 점이 생겼는데,
잘못된 값을 입력받았을 때 다시 입력받도록 처리하다 보니 Controller의 코드가 길어져, 한눈에 파악하기 어렵다는 문제가 있었습니다.
Controller도 Domain처럼 여러 클래스로 분리할까 고민했지만, 어차피 Model과 View를 연결하는 역할이라 굳이 그렇게까지 분리해야 할까 하는 생각이 들어 현재는 하나로 유지했습니다. 규모가 큰 프로젝트에서는 Controller도 여러 클래스로 분리하는지 궁금합니다.

Copy link

@gogo1414 gogo1414 left a comment

Choose a reason for hiding this comment

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

안녕하세요, 고은님!
리뷰 반영하시느라 고생하셨습니다 😊

추석 연휴는 잘 보내고 계실지 모르겠네요! 아직 연휴가 많이 남았으니 남은 기간 푹 쉬고 하고 싶은 일을 하는 시간이 되셨으면 좋겠습니다 👍

간단한 코멘트

네이밍 통일

  • Lotto, LottoTicket 둘 중 하나의 클래스명으로 통일해주신거 잘 확인했습니다! 다만, 변수명은 아직 통일되지 않은 형태로 사용되는 것으로 보여요! 해당 부분까지 같이 신경써주시면 좋을 것 같아요!

질문에 대한 답변

잘못된 값을 입력받았을 때 다시 입력받도록 처리하다 보니 Controller의 코드가 길어져, 한눈에 파악하기 어렵다는 문제가 있었습니다.
Controller도 Domain처럼 여러 클래스로 분리할까 고민했지만, 어차피 Model과 View를 연결하는 역할이라 굳이 그렇게까지 분리해야 할까 하는 생각이 들어 현재는 하나로 유지했습니다. 규모가 큰 프로젝트에서는 Controller도 여러 클래스로 분리하는지 궁금합니다.

  • 여러 개의 도메인이 생기는 경우에 Controller가 각 도메인에 맞게 여러개를 생성(ex, UserController, LottoController)하기도 하지만, 지금과 같이 동일한 도메인에서 긴 코드에 대한 우려가 생길 경우에는 반복되는 입력 및 검증 로직을 별도의 private 메서드로 추출해서 적용해보는 방식도 적용할 수 있어요!
  • 현재 미션에서는 여러개의 컨트롤러로 분리될 경우 코드 파악이 어려워질 것 같아요 🥲

이번에 수정하신 내용과 답글을 보며, 도메인 계층의 역할이라는 주제에 대해 함께 이야기 나눠보면 좋겠다는 생각이 들었어요. 아래는 이 주제에 대한 제 생각을 정리해본 것인데, 고은님의 의견도 궁금합니다!

도메인 계층의 역할

제가 생각하는 도메인 계층은 크게 두 가지 종류의 객체로 구성될 수 있습니다.

도메인 객체

  • 도메인 객체는 Lotto처럼 비즈니스의 핵심 개념을 표현하며, 관련된 데이터와 그 데이터를 직접 사용하는 로직(예: 번호 개수 유효성 검사)을 함께 가집니다. 객체가 자신의 상태를 스스로 책임지는 것이죠.

도메인 서비스

  • 도메인 서비스는 CreateLotto가 하려 했던 역할처럼, 특정 객체 하나의 책임이라고 보기 어려운 행위를 담당합니다. '자동으로 로또 번호를 생성하는' 로직이 좋은 예시가 될 수 있습니다.

로직을 분리하려는 고은님의 직관은 매우 좋다고 생각해요. 만약 제가 이 개념들을 바탕으로 리팩토링 방향을 잡아본다면,

  • Lotto는 스스로를 검증하는 로직을 가진 도메인 객체로 강화하고,
  • 번호 생성 로직은 LottoGenerator와 같은 도메인 서비스로 분리한 뒤,
  • 흩어져 있는 결과 계산 로직들(MatchCount, ProfitRate 등)을 LottoResult라는 하나의 도메인 객체로 통합할 것 같습니다.

이런 식으로 각 클래스의 역할을 재구성하는 것에 대해 어떻게 생각하시나요? 고은님께서 처음 CreateLotto를 만드셨을 때의 의도와 비교해서 어떤 점이 비슷하고 다른지 궁금하네요!

Comment on lines +18 to +22
public void validateManualCount(int manualCount) {
if (manualCount < 0 || manualCount > count) {
throw new IllegalArgumentException("수동으로 구매할 로또 수는 0 이상이여야 하며, 구매 가능한 티켓 수를 초과할 수 없습니다.");
}
}
Copy link

Choose a reason for hiding this comment

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

수동으로 구매할 로또 수를 검증하는 로직이 LottoTicketCount 객체에 존재하는 이유가 있을까요??

Copy link
Author

Choose a reason for hiding this comment

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

처음에 검증 로직 넣으면서, 수동 구매 수는 0이상에 총 로또 수 보다 작아야 해서 총 로또 수는 변수로 가지고 있는 클래스들을 살펴보다가 클래스 이름도 TicketCount로 포괄적인 것 같아 여기에 넣는게 자연스러울 것 같아서 LottoTicketCount에 넣어두었습니다!

Comment on lines +53 to +60
SortedSet<LottoNumber> duplicateNumbers = new TreeSet<>(Arrays.asList(
new LottoNumber(1),
new LottoNumber(2),
new LottoNumber(3),
new LottoNumber(4),
new LottoNumber(5),
new LottoNumber(5)
));
Copy link

@gogo1414 gogo1414 Oct 6, 2025

Choose a reason for hiding this comment

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

MatchCountTest에 존재하는 createLotto 로직을 별도로 활용할 수 있는 방법이 없을까요?

로또 미션의 테스트 코드 특성 상 Lotto 객체를 메 테스트마다 생성해줘야 하는 것 같은데 별도의 좋은 방법은 없을까요?

한번 고민해보시면 좋을 것 같아요!

Copy link
Author

Choose a reason for hiding this comment

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

이 부분 공부를 해 보았는데 현재 테스트 코드는 중복된 번호가 포함된 케이스를 테스트 하는 것이라 내장된 _함수(?) 라고 칭해도 될까요..._인 @beforeach 를 사용하는게 제일 적절해 보였습니다.
하지만 그렇게 되면 매 테스트 마다 객체를 새로 생성하게 되는데, 이렇게 되면 결국 처음 고민했던 객체 생성 중복 문제를 그대로 가져오는 게 아닌가 라는 생각이 들어서요..!
이 부분 한번 날 잡고 깊게 고민해 보겠습니다 ㅜㅜ!! 🥲🥲

Comment on lines 21 to 23
public SortedSet<LottoNumber> getNumbers() {
return new TreeSet<>(numbers);
}
Copy link

Choose a reason for hiding this comment

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

List 자료구조도 존재하고 해당 자료구조에도 정렬할 수 있는 로직이 존재하는데, 타입을 Set으로 선택하신 이유도 있을까요?

Lotto 객체 생성 특성상 한번 생성된 이후로 번호가 변경되거나 새로 삽입되지 않아서 이유가 궁금하네요!

int match3Count = matchCount.getCount(LottoPrice.MATCH_3);
int match4Count = matchCount.getCount(LottoPrice.MATCH_4);
int match5Count = matchCount.getCount(LottoPrice.MATCH_5);
int matchSecond = matchCount.getCount(LottoPrice.MATCH_5_BONUS);
Copy link

@gogo1414 gogo1414 Oct 6, 2025

Choose a reason for hiding this comment

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

설명해주셔서 감사해요!
다만, matchSecond라는 네이밍을 처음 봤을 때 저는 2개만 맞은 걸로 파악이 되었어요..😅

형식을 유지해서 match5CountWithBonus라는 형태 혹은 네이밍 전체를 바꿔 1등부터 5등을 표현하는 네이밍으로 변경하면 이해하기 더 편할 것 같다는 생각이 드네요!

assertThrows(IllegalArgumentException.class, () -> new Lotto(duplicated));
@DisplayName("로또 번호 수동 입력 시 중복된 숫자가 들어올 경우 예외가 발생한다.")
void manualLottoDuplicate() {
SortedSet<LottoNumber> duplicateNumbers = new TreeSet<>(Arrays.asList(new LottoNumber(1), new LottoNumber(2), new LottoNumber(3), new LottoNumber(4), new LottoNumber(5), new LottoNumber(5)));
Copy link

@gogo1414 gogo1414 Oct 6, 2025

Choose a reason for hiding this comment

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

지금은 인텔리제이의 자동정렬로 충분할 수 있지만, 협업 혹은 직무에 나가서는 특정 코드 컨벤션을 정하고 해당 컨벤션에 맞게 작업을 하실 수도 있어요!

만약, 시간이 되신다면 코드 컨벤션에 관한 내용을 함께 찾아보면 많은 도움이 되실 것 같습니다 😄

@ke-62
Copy link
Author

ke-62 commented Oct 9, 2025

먼저, 리뷰가 늦어진 점 너무너무 죄송합니다ㅜㅜ! 연휴에 추가적으로 이런저런 일이 많이 생겨 그것들 처리하느라 너무 바빠졌습니다. 다음부터는 꼭.. 늦지 않고 리뷰 반영해서 올리도록 하겠습니다. 🥹

이번 리뷰에서는 도메인 객체와 도메인 서비스에 관해 깊게 생각해 보게 된 것 같습니다.
돌이켜보니 제 코드에서 클래스가 유달리 많았던 이유, 그리고 클래스들의 위치와 역할이 종종 헷갈렸던 이유가 이런 개념을 명확히 이해하지 못했기 때문이였던 것 같습니다!!
말씀해 주신 대로 분산되어 있는 MathCount,ProfitRate를 LottoReult나 LottoMaker 의 네이밍을 붙인 새로운 객체로 통합하면, 도메인을 훨씬 명확하게 표현할 수 있을 것 같습니다..!
오늘 기준으로 시험이 약 10일정도 밖에 남지 않아 이 관련 리팩토링은 시험 끝나고 바로 진행하도록 하겠습니다. 😿😿
항상 감사합니다 !!

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