Skip to content

[완두콩] 조하은 로또 미션 제출합니다. - #205

Merged
boorownie merged 28 commits into
next-step:johaeunnfrom
johaeunn:step5
Aug 8, 2026
Merged

[완두콩] 조하은 로또 미션 제출합니다.#205
boorownie merged 28 commits into
next-step:johaeunnfrom
johaeunn:step5

Conversation

@johaeunn

@johaeunn johaeunn commented Aug 2, 2026

Copy link
Copy Markdown

안녕하세요, 김병웅 리뷰어님. 완두콩 스터디원으로 참여하고 있는 조하은입니다.
아직 코드에 부족한 부분이 많겠지만, 이번 미션 잘 부탁드립니다! 🙇‍♀️

현재 학습 상황

3년전, Java를 배운 이후 자주 사용하지 않아 현재는 기본 문법부터 다시 익히고 있습니다.
Spring 프로젝트를 진행한 경험은 있지만 당시 구현에 AI의 도움을 많이 받아, 스스로 코드를 설계하고 구현하는 경험은 아직 부족한 편입니다. 이번 스터디를 통해 Java 기초와 객체 설계 능력을 배워가고 있습니다.

참고 사항

  • 로또 1장의 가격이 1,000원이므로 최소 구입 금액을 1,000원으로 정하여 구현하였습니다.
    별도의 요구사항에 명시된 내용은 아니어서 이와 같은 예외 처리가 적절한지도 궁금합니다.
  • 1단계 힌트에 로또 자동 생성 시 Collections.shuffle()을 활용한다고 되어 있었지만, 어떻게 사용하는 것이 좋을지 감이 잡히지 않아 현재는 랜덤으로 번호를 하나씩 생성하고 중복을 확인하는 방식으로 구현하였습니다.
    Collections.shuffle()을 사용하는 것이 이번 미션에서 의도한 구현 방식인지, 현재와 같은 방식으로 구현해도 괜찮은지 궁금합니다.

이번 미션에서 어려웠던 부분

1. 원시 값을 객체로 포장하고 책임을 나누는 과정

2단계에서 모든 원시 값과 문자열을 포장하는 과정이 가장 어려웠습니다.
특히 LottoNumber를 추가하면서 기존에 int로 사용하던 로또 번호를 객체로 변경하는 데 가장 많은 시간이 들었습니다.

처음에는 단순한 숫자를 굳이 객체로 포장해야 하는 이유가 잘 와닿지 않았고,
Lottoint 값이 아닌 LottoNumber 객체를 이용하도록 변경하는 것도 익숙하지 않았습니다.

처음부터 어떤 부분을 함께 수정해야 하는지 파악하기 어려워 우선 LottoNumber를 적용한 뒤,
그로 인해 발생하는 컴파일 오류와 IntelliJ에서 표시해 주는 오류를 하나씩 확인하며 관련된 코드를 하나하나 수정해 나갔습니다.

구현을 진행하면서 현재는 로또 번호의 범위와 같은 규칙을 LottoNumber에서 검증하도록 함으로써,
각 객체가 자신의 상태와 관련된 규칙을 관리할 수 있다는 점이 원시 값을 포장하는 이유 중 하나라고 이해하였습니다.
이와 같이 제가 이해한 방향이 적절한지 궁금합니다.

2. 테스트하기 좋은 코드를 만드는 방법

3단계까지는 Lotto가 랜덤으로만 생성되었기 때문에 특정 번호를 가진 로또를 만들어 당첨 횟수 집계나 수익률 계산을 테스트하기 어려웠습니다.

당시에는 랜덤 번호 생성을 인터페이스로 분리해야 하는지 고민하였는데,
4단계에서 수동 로또 기능을 구현하면서 Lotto 생성자를 오버로딩하여원하는 번호로 Lotto를 생성할 수 있게 되었고, 이를 통해 통계 관련 테스트도 작성할 수 있었습니다.

다만 현재처럼 생성자 오버로딩을 이용하는 방식이 적절한 설계인지,
혹은 랜덤 번호 생성 자체를 별도의 책임으로 분리하는 것이 더 좋은 방법인지 궁금합니다.

3. LottoStatistics의 책임과 메서드 분리

처음 LottoStatistics를 구현할 때는 당첨 번호 비교, 보너스 번호 확인, 당첨 횟수 증가 등을 하나의 메서드 안에서 처리하려고 해서 메서드 길이와 depth가 많이 증가하였습니다.

이후 LottoRank를 Enum으로 변경하면서 등수와 상금에 대한 정보를 Enum에서 관리하도록 수정했고,
당첨 결과는 EnumMap<LottoRank, Integer>로 관리하도록 변경했습니다.

그 과정에서 기존보다 조건문과 상금 계산 로직을 줄일 수 있었고, 메서드를 역할별로 분리하면서 코드의 흐름도 조금 더 명확해졌다고 느꼈습니다.

리뷰에서 중점적으로 봐주셨으면 하는 부분

1. Application의 역할과 메서드 길이

현재 Application은 입력부터 도메인 객체 생성, 결과 출력까지 전체 실행 흐름을 담당하고 있어
다른 메서드에 비해 코드가 길게 작성되어 있습니다.

프로그래밍 요구사항의 "함수(또는 메서드)의 길이를 10라인 이하로 구현한다"는 기준을
main 메서드에도 동일하게 적용하는 것이 좋은지 궁금합니다.

10라인을 맞추기 위해 단순히 한두 줄짜리 메서드를 많이 만드는 것도
오히려 흐름을 읽기 어렵게 만들 수 있다고 생각해서,
Application을 어느 정도까지 분리하는 것이 적절한지 리뷰 부탁드립니다.

2. OutputView에서 LottoStatistics를 생성하는 것이 적절한지

현재 OutputViewprintLottoStatistics() 내부에서 LottoStatistics 객체를 직접 생성한 뒤 당첨 통계와 수익률을 출력하도록 구현했습니다.

구현 당시에는 통계를 출력하는 과정에서 필요한 객체라고 생각해 OutputView에서 생성하였지만,
지금 생각해보니 View가 단순히 출력만 담당하는 것이 아니라 도메인 객체 생성까지 책임지게 하는 것인가 고민이 들었습니다.
그래서 LottoStatisticsApplication에서 생성하도록 옮기는 것이 더 적절한지 궁금합니다.

3. 클래스 내부의 메서드 배치 순서

구현하면서 클래스 내부에서 메서드를 어떤 순서로 배치하는 것이 읽기 좋은 코드인지 고민이 있었습니다.

현재는 public 메서드를 먼저 배치하고 private 메서드를 아래에 모아두는 방식을 주로 사용했는데,
호출되는 흐름에 따라 관련 메서드를 가까이 배치하는 것이 더 읽기 좋은 경우도 있을 것 같았습니다.

예를 들어 하나의 public 메서드가 여러 private 메서드를 호출할 때,
접근 제어자별로 메서드를 모아두는 것이 좋은지,
아니면 실제 코드의 실행 흐름을 따라 관련 메서드를 배치하는 것이 좋은지 궁금합니다.

메서드 배치에도 일반적으로 권장되는 기준이나,
현재 코드에서 가독성을 위해 개선하면 좋을 부분이 있다면 리뷰 부탁드립니다.

4. 클린 코드 학습과 리팩토링

이번 미션의 학습 테스트에서 다루는
'읽기 좋은 코드', '예측 가능한 코드', '실수를 방지하는 코드'에 대한 내용을 아직 충분히 학습하지 못한 상태에서 구현을 진행하였습니다.

이에 기능 구현과 기본적인 리팩토링은 진행하였지만, 학습 테스트의 내용을 기준으로 코드를 다시 충분히 다듬어보지는 못했습니다.
이번 리뷰를 통해 제가 놓치고 있는 클린 코드 관점이나 추가로 고민해보면 좋을 부분이 있다면 알려주시면 감사하겠습니다.

긴 글 읽어주시고, 소중한 시간 내어 리뷰해 주셔서 감사합니다!

@quddaz quddaz left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

안녕하세요! 로또 미션 구현하시느라 고생하셨습니다. 😊

전체적으로 로또 번호, 구매 금액, 당첨 통계 등 각 개념을 객체로 분리하려고 고민한 점이 좋았습니다. README에 구현 과정에서 고민한 내용을 구체적으로 작성해 주셔서 설계 의도도 쉽게 파악할 수 있었습니다.

남겨주신 질문에 대해 먼저 답변드리겠습니다.

  1. main()에도 10라인 규칙을 적용하되, 줄 수를 맞추기 위해 무조건 메서드를 분리할 필요는 없다고 생각합니다. 구매, 당첨 정보 입력, 통계 출력처럼 하나의 의미 있는 실행 단계를 기준으로 분리해 보세요. 메서드 이름만 읽어도 전체 흐름을 파악할 수 있는지가 좋은 기준이 될 것 같습니다.

  2. 고민해 주신 것처럼 LottoStatisticsOutputView 외부에서 생성하는 편이 역할을 더 명확하게 구분할 수 있습니다. Application이 객체를 생성하고 연결하며, OutputView는 완성된 결과를 전달받아 출력하도록 구성해 보는 것을 권장드립니다. 자세한 내용은 해당 코드에 코멘트를 남겼습니다.

  3. 메서드 배치 순서에는 하나의 정답이 있기보다 팀의 컨벤션에 따라 달라질 수 있습니다. 현재처럼 외부에서 사용하는 public 메서드를 먼저 두고, 해당 메서드가 호출하는 private 메서드를 호출 흐름에 따라 가까이 배치하면 충분히 읽기 좋은 구조라고 생각합니다. 접근 제어자별로 기계적으로 모으기보다 위에서 아래로 자연스럽게 읽히는지를 기준으로 살펴보면 좋겠습니다.

  4. 클린 코드는 정해진 형태를 한 번에 완성하는 것보다, 현재 코드에서 변경하기 어렵거나 실수하기 쉬운 부분을 발견하고 개선하는 과정에서 학습할 수 있다고 생각합니다. 이번 리뷰에서는 객체가 생성된 순간부터 유효한 상태인지, 객체가 자신의 책임에 집중하고 있는지, 테스트가 실제 동작을 충분히 검증하는지를 중심으로 코멘트를 남겼습니다.

이번 리뷰에서 주로 살펴본 내용은 다음과 같습니다.

  • 1,000원 단위가 아닌 구매 금액의 처리 정책
  • OutputView가 통계 객체를 생성하는 구조
  • 로또 번호의 랜덤 생성 책임
  • 객체가 항상 유효한 상태를 유지하는 방법
  • 미당첨 결과를 null로 표현했을 때의 영향
  • 수동·자동 로또 생성 테스트의 검증 범위

모든 코멘트를 정답처럼 그대로 반영하기보다는, 현재 구조에서 어떤 문제가 발생할 수 있는지와 각 객체가 어떤 책임을 가져야 하는지를 기준으로 고민해 보시면 좋겠습니다.

코멘트를 확인한 뒤 궁금하거나 의견을 나누고 싶은 부분이 있다면 편하게 남겨주세요!

Comment on lines +39 to +43
private void validatePurchaseAmount(int amount) {
if (amount < LOTTO_PRICE) {
throw new IllegalArgumentException("최소 구입 금액은 " + LOTTO_PRICE + "원 입니다.");
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

현재 1,000원 미만의 금액은 거절하지만, 1,500원처럼 로또 가격으로 나누어떨어지지 않는 금액은 허용하고 있습니다. 이 경우 한 장만 발급되고 남은 500원은 별도의 안내 없이 사라집니다.

구입 금액을 반드시 1,000원 단위로 받으려는 것인지, 남은 금액을 허용하려는 것인지 정책을 먼저 정해보면 좋겠습니다.

미션의 현재 규모에서는 amount % LOTTO_PRICE == 0인지 검증하는 방식이 가장 단순해 보이지만, 중요한 것은 검증 코드 자체보다 프로그램이 어떤 금액을 유효한 구입 금액으로 보는지 설명할 수 있는 것이라고 생각합니다. 😆

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

구입 금액이 1,000원 단위가 아닌 경우 나머지 금액이 발생한다는 점은 알고 있었으나, 이를 단순히 남는 금액의 문제로만 생각하고 프로그램에서 어떤 금액을 유효한 구입 금액으로 허용하지 정책을 정해야 한다는 점까지는 생각하지 못했습니다..

말씀해주신 내용을 통해 이러한 부분도 명확한 기준을 정하고 표현해야 한다는 것을 알게 되었고, 구입 금액도 1,000원 단위로 제한하는 것이 자연스럽다고 판단하여 수정하였습니다!

Comment thread src/main/java/view/OutputView.java Outdated
Comment on lines +26 to +34
public void printLottoStatistics(Lottos lottos, WinningNumbers winningNumbers,
PurchaseAmount purchaseAmount, LottoNumber bonusNumber) {
printStatisticsHeader();

LottoStatistics lottoStatistics = new LottoStatistics(lottos, winningNumbers, bonusNumber);
printWinningCounts(lottoStatistics);

printProfitRate(lottoStatistics, purchaseAmount);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

고민해 주신 것처럼 LottoStatisticsOutputView 외부에서 생성하는 편이 역할을 더 명확히 보여줄 수 있을 것 같습니다.

현재 OutputView는 결과를 출력하는 것뿐 아니라, 어떤 데이터로 통계를 만들고 언제 계산할지도 결정하고 있습니다. 이로 인해 출력 방식의 변경이 도메인 객체를 생성하는 흐름에도 영향을 줄 수 있어요.

Application에서 통계를 생성한 뒤 OutputView에는 완성된 결과를 전달하면 다음과 같이 역할을 구분할 수 있습니다.

  • Application: 프로그램 흐름을 제어하고 객체를 연결
  • LottoStatistics: 당첨 결과와 수익률 계산
  • OutputView: 전달받은 결과 출력

단순히 코드 위치를 옮기는 것보다, View가 도메인 계산의 시작 시점까지 결정해도 되는지를 기준으로 고민해 보면 좋겠습니다. 😊

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

처음에는 통계를 출력하기 위해 필요한 객체이기 때문에 OutputView에서 생성해도 된다고 생각했습니다. 하지만 LottoStatistics가 생성되는 순간 당첨 결과 집계까지 수행된다는 점을 미처 생각하지 못하였습니다. 결과적으로 현재 코드는 OutputView가 단순히 결과를 출력하는 것 뿐만 아니라 도메인 계산을 시작하는 시점까지 결정하고 있다는 것을 알게 되었습니다.

이에 View는 전달받은 결과를 표현하는 역할에 집중하는 것이 더 적절하다고 생각하여 LottoStatistic의 생성을 Application으로 이동하였습니다!

Comment thread src/main/java/domain/Lotto.java Outdated
Comment on lines +16 to +27
public Lotto() {
makeNumbers();
}

public Lotto(List<Integer> numberValues) {
validateNumberCount(numberValues);
validateDuplicateNumbers(numberValues);

convertToLottoNumbers(numberValues);

Collections.sort(numbers);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

현재 Lotto는 로또 번호의 개수와 중복을 검증하는 책임뿐 아니라, 랜덤 번호를 생성하는 방식까지 알고 있습니다.

생성자 오버로딩 자체가 문제라기보다 new Lotto()만으로는 랜덤 생성이 수행된다는 사실을 예측하기 어렵고, 테스트에서 원하는 자동 로또를 만들기도 어렵다는 점을 생각해 볼 수 있을 것 같아요.

자동 번호 생성을 LottoGenerator와 같은 별도의 객체로 분리하고, Lotto는 전달받은 번호가 유효한지만 관리하도록 구성하는 방법은 어떨까요?

Collections.shuffle()을 사용하는지는 구현 방법의 선택입니다. 현재처럼 중복을 확인하며 번호를 하나씩 뽑아도 기능상 문제는 없습니다. 다만 번호 생성 방식과 로또가 지켜야 하는 규칙을 같은 책임으로 볼 것인지는 구분해서 고민해 보면 좋겠습니다.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

기존에는 new Lotto()를 호출하면 자동으로 랜덤 번호가 생성되도록 구현하였는데, 말씀해주신 것처럼 코드에 대한 사전 지식이 없다면 단순한 객체 생성인지 랜덤 생성까지 수행되는 것인지 예측하기 어렵다는 점을 생각하지 못했습니다.

또한 로또 번호가 지켜야 하는 도메인 규칙과 랜덤 번호를 생성하는 방식은 서로 다른 책임이라고 생각하여 자동 생성 책임을 LottoGenerator로 분리하였습니다. 이에 Lotto는 전달받은 번호의 개수와 중복 등을 검증하고 유효한 로또를 생성하는 역할만 담당하도록 수정하였습니다.

LottoGenerator에서는 랜덤한 List<Integer>를 생성한 뒤 new Lotto(numbers)를 통해 로또를 생성하도록 하여 자동 로또와 직접 지정한 로또 모두 동일한 Lotto 생성자를 거치도록 수정했습니다. 또한 호출하는 쪽에서도 new Lotto() 대신 lottoGenerator.generate()를 사용하여 자동 생성이 수행된다는 의도가 조금 더 명확하게 드러나도록 하였습니다.

또한 자동 생성과 관련된 테스트는 LottoGeneratorTest로 이동하여 Lotto의 검증 테스트와 분리하였습니다. 이번에는 우선 랜덤 번호 생성 책임을 LottoGenerator로 분리하였으며, 이후 자동 생성 결과 자체를 제어해야 하는 테스트가 필요해질 경우 생성기를 외부에서 전달 받는 구조도 함께 고려해보겠습니다!

Comment thread src/main/java/Application.java Outdated
Comment on lines +28 to +31
WinningNumbers winningNumbers = new WinningNumbers(inputView.readWinningNumbers());
LottoNumber bonusNumber = new LottoNumber(inputView.readBonusNumber());

winningNumbers.validateBonusNumber(bonusNumber);

@quddaz quddaz Aug 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

현재 WinningNumbers를 생성한 뒤, Application에서 별도로 validateBonusNumber()를 호출하고 있습니다.
이 구조에서는 객체를 사용하는 쪽이 반드시 검증 호출 순서를 기억해야 합니다. 새로운 호출자가 검증을 빠뜨리면 당첨 번호와 보너스 번호가 중복된 상태로 통계 계산이 진행될 수도 있어요.
당첨 번호와 보너스 번호를 하나의 당첨 정보로 본다면, 두 값을 함께 받아 생성 시점에 검증하는 WinningLotto와 같은 객체로 구성할 수도 있을 것 같습니다.

WinningLotto winningLotto =
        new WinningLotto(winningNumbers, bonusNumber);

객체 이름보다 중요한 것은 당첨 번호와 보너스 번호를 함께 사용하는 시점에 중복 검증이 반드시 수행되도록 보장하는 것입니다. 객체가 생성된 직후부터 항상 유효한 상태인지라는 기준으로 고민해 보면 좋겠습니다. 😊

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

기존에는 Application에서 WinningNumbersbonusNumber를 생성한 뒤 validateBonusNumber()를 호출하면 된다고 생각하였습니다. 하지만 이 방식에서는 객체를 사용하는 쪽이 검증 호출을 반드시 기억해야 하고, 새로운 호출자가 검증을 빠뜨릴 경우 당첨 번호와 보너스 번호가 중복된 상태로도 통계 계산이 진행될 수 있다는 점까지는 생각하지 못했습니다.

말씀해주신 것처럼 당첨 번호와 보너스 번호를 하나의 당첨 정보로 보는 것이 자연스럽다고 생각하여 WinningLotto 객체를 추가하였습니다. WinningLotto 생성 시 두 값을 함께 받아 중복 여부를 검증하도록 하여 별도로 검증 메서드를 호출하지 않아도 생성 시점부터 유효한 당첨 정보를 가지도록 수정하였습니다.

또한 기존에 WinningNumbers winningNumbers, LottoNumber bonusNumber를 각각 전달하던 부분도 WinningLotto winningLotto 하나를 전달하도록 변경하였습니다.

수정하는 과정에서 기존 LottoStatisticsprocessLottoResult()는 당첨 번호 일치 개수와 보너스 번호 일치 여부를 각각 구한 뒤 등수를 결정하고 있었습니다. 두 정보 모두 WinningLotto가 가지고 있기 때문에 외부에서 각각의 값을 이용하여 등수를 판단하기보다는 WinningLotto 안에서 한 장의 로또에 대한 등수를 판단하도록 하는 것이 더 자연스럽다고 생각하여 determineRank()를 추가하였습니다.
이처럼 수정한 결과 LottoStatistics는 반환된 등수를 집계하는 역할에 집중하도록 수정되었습니다.

WinningLotto가 당첨 정보의 유효성을 보장하는 것뿐 아니라 한 장의 로또 등수까지 판단하도록 한 책임 분리가 적절한 방향인지 궁금합니다!

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

WinningLotto가 당첨 번호와 보너스 번호의 유효성을 보장하고, 한 장의 로또에 대한 등수를 판단하도록 한 방향은 자연스럽다고 생각합니다. WinningLotto가 당첨 정보와 등수 판단에 필요한 규칙을 함께 알고 있기 때문입니다. 👍

다만 WinningLotto가 직접 일치 개수를 계산하는 것까지 반드시 담당해야 하는지는 한 번 더 고민해보면 좋겠습니다.

“두 로또의 번호가 몇 개 일치하는가”는 Lotto끼리의 비교에 가까운 책임이므로, 예를 들어 Lotto.countMatchingNumbers(Lotto other)처럼 Lotto에 둘 수 있습니다. 이후 WinningLotto는 해당 결과와 보너스 번호를 바탕으로 등수를 판단하는 역할을 맡을 수 있습니다.

정리하면 다음과 같이 나눌 수 있을 것 같아요.

  • Lotto: 다른 로또와 일치하는 번호 개수 계산
  • WinningLotto: 당첨 번호·보너스 번호를 포함한 당첨 판정
  • LottoStatistics: 판정된 등수 집계

현재처럼 determineRank()WinningLotto에 둔 것은 적절해 보이며, 내부의 일치 개수 계산 책임까지 어디에 둘지는 객체 간 데이터 노출과 응집도를 기준으로 추가로 고민해보면 좋겠습니다. 😊

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

말씀해주신 내용을 바탕으로 일치 개수 계산 책임을 옮기는 방향을 고민하면서 WinningNumbers의 구조를 다시 살펴보았습니다.

이 과정에서 WinningNumbers가 내부적으로 List<LottoNumber>를 관리하고 있는데, Lotto 역시 동일하게 List<LottoNumber>를 관리하고 있다는 점이 눈에 들어왔습니다. 두 객체가 어떤 차이를 가지고 있는지 다시 비교해보니 모두 1~45 사이의 중복되지 않는 6개의 번호를 가진다는 동일한 규칙을 가지고 있었습니다.

기존에는 WinningNumberscountMatchingNumbers()가 있어 별도의 역할이 있다고 생각했지만, 리뷰해주신 것처럼 이 행동 역시 두 로또의 번호를 비교하는 책임으로 볼 수 있어 Lotto로 옮길 수 있다고 판단하였습니다. 그러고 나니 WinningNumbers를 별도의 객체로 유지해야 할 이유가 크지 않다고 생각하여 당첨 번호도 Lotto로 표현하도록 변경하였습니다.

함께 살펴보면서 보너스 번호 포함 여부도 getNumbers()로 번호 목록을 꺼내 외부에서 확인하기보다 Lotto가 직접 판단하도록 변경하였습니다. 현재는 Lotto가 번호 간의 비교와 특정 번호의 포함 여부를 담당하고, WinningLotto는 당첨 번호와 보너스 번호를 바탕으로 최종 등수를 판단하도록 역할을 나누었습니다.

말씀해주신 객체 간 데이터 노출과 응집도를 기준으로 고민하다 보니 기존에 중복되어 있던 객체의 역할도 함께 정리하게 되었는데, 이런 방향으로 수정한 것이 적절한지 궁금합니다!

Comment thread src/main/java/domain/LottoRank.java Outdated
if (matchCount == 5) return THIRD;
if (matchCount == 4) return FOURTH;
if (matchCount == 3) return FIFTH;
return null;

@quddaz quddaz Aug 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LottoRank.from()은 당첨되지 않은 경우 null을 반환하고 있습니다.

미당첨도 로또 결과에서 정상적으로 발생하는 상태인데 null로 표현하면, 호출하는 모든 곳에서 별도의 방어 코드를 작성해야 합니다. 또한 검사를 빠뜨리면 NullPointerException으로 이어질 수 있어요.

MISS 또는 NONE과 같은 미당첨 상태를 LottoRank에 포함하는 방법도 생각해 볼 수 있을 것 같습니다.

다만 MISS까지 통계 출력 대상에 포함되면 LottoRank.values()를 순회하는 부분에서 제외 처리가 필요할 수 있습니다. null 방어 비용과 미당첨 상태를 Enum에 포함했을 때의 영향을 비교해 보면 좋겠습니다.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

말씀해주신 내용을 바탕으로 null을 유지했을 때 발생하는 방어 비용과, MISS를 Enum에 포함했을 때의 영향을 비교해보았습니다.

null 유지 MISS 추가
LottoRank에는 당첨 등수만 존재 미당첨까지 로또 결과로 명시 가능
from()null을 반환할 수 있음 from()이 항상 LottoRank를 반환
호출하는 곳에서 null 검사 필요 null 검사 불필요
검사 누락 시 NullPointerException 발생 가능 null로 인한 NullPointerException 위험 감소
values() 순회 시 별도 제외 처리 불필요 당첨 등수만 필요한 경우 MISS 제외 필요

비교해본 결과, 미당첨 역시 정상적으로 발생하는 로또 결과이므로 null보다는 MISS라는 상태로 명시하는 것이 더 자연스럽다고 판단하였습니다. 또한 null을 반환할 경우 이를 사용하는 곳마다 별도의 방어 코드가 필요하고, 추후 코드가 추가되거나 수정될 때마다 null 가능성을 계속 고려해야 한다는 점에서도 유지보수 비용이 더 크다고 생각하였습니다.

이에 LottoRank에 MISS를 추가하고 당첨 등수만 필요한 경우 사용할 수 있도록 winningRanks()를 추가하였습니다. 출력에서는 .values() 대신 .winningRanks()를 순회하도록 하여 MISS가 출력되지 않도록 수정하였습니다.

다만 수정하는 과정에서 현재 LottoRankmatchCountprize를 가지고 있는데, MISS는 0개, 1개, 2개가 일치하는 상황이 모두 포함되기 때문에 MISS(0, 0)으로 표현하는 것이 적절한지 의문이 들었습니다. 단순히 0을 넣어도 되는지, 아니면 다른 방식으로 표현하는 것이 더 적절한지 궁금합니다.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

MISS를 추가한 방향은 자연스럽다고 생각합니다. 미당첨도 로또 결과 중 하나이므로 null보다 명시적인 상태로 표현할 수 있고, 호출하는 쪽의 방어 코드도 줄어들겠네요. 👍

다만 말씀하신 것처럼 MISS(0, 0)은 조금 고민해볼 필요가 있습니다. 미당첨은 0개뿐만 아니라 1개, 2개가 일치한 경우도 포함하기 때문에 matchCount = 0이 실제 미당첨 조건 전체를 표현하지는 못하기 때문입니다.

현재 matchCount가 등수를 판별하기 위한 기준으로만 사용되고, MISS의 실제 일치 개수를 조회하지 않는다면 MISS(0, 0)을 일종의 대표값으로 사용하는 것도 가능합니다. 다만 이 값이 “미당첨은 항상 0개 일치”라는 의미로 해석되지 않도록 주의해야 합니다.

반대로 matchCount가 통계나 출력에 사용된다면 MISS에 임의의 0을 넣기보다는, 당첨 등수 정보와 미당첨 상태를 분리하거나 matchCount를 별도로 관리하는 방법도 고민해볼 수 있습니다.

즉 현재 구조에서는 MISS(0, 0)을 사용해도 되지만, 0이 미당첨의 모든 경우를 의미하는 값은 아니라는 점을 명확히 하고 사용하는 것이 중요해 보입니다. 😊


Lottos lottos = new Lottos(manualLottoNumbers, autoLottoCount);

assertEquals(12, lottos.getLottos().size());

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

현재 테스트는 수동 로또와 자동 로또를 합한 최종 개수만 검증하고 있습니다.

따라서 수동 번호가 실제 구매 결과에 포함되지 않고, 같은 개수의 자동 로또만 생성되더라도 테스트가 통과할 수 있어요.

입력한 수동 로또가 최종 결과에 포함됐는지도 함께 검증하면 테스트 이름에서 설명하는 핵심 동작을 더 정확히 확인할 수 있을 것 같습니다.

다만 자동 로또 결과는 현재 제어할 수 없으므로, 수동 로또의 포함 여부부터 검증하거나 앞에서 이야기한 생성 책임 분리와 함께 테스트 방법을 고민해 보면 좋겠습니다. 😊

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

기존 테스트에서는 수동 로또와 자동 로또를 합한 최종 개수만 확인하고 있어서 말씀해주신 것처럼 수동 로또가 실제 구매 결과에 포함되지 않아도 테스트가 통과할 수 있다는 점을 놓쳤습니다..

이에 최종 로또 개수뿐만 아니라 입력한 수동 로또 번호들이 실제 구매 결과에 모두 포함되어 있는지도 함께 검증하도록 테스트를 보완하였습니다!

또한 앞선 리뷰를 반영하여 자동 번호 생성 책임을 LottoGenerator로 분리하였습니다. 다만 현재 Lottos가 내부에서 LottoGenerator를 생성하고 있어 테스트에서 자동 생성 결과 자체를 원하는 값으로 제어할 수는 없는 상태입니다.

말씀해주신 생성 책임 분리와 함께 테스트 방법을 고민해보라는 부분이 생성기를 외부에서 주입받는 등의 방식으로 자동 생성 결과까지 제어할 수 있는 테스트 구조를 구성해보는 방향을 추천해주신 것인지 궁금합니다.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

네, 맞습니다. 수동 로또 번호가 실제 구매 결과에 포함되는지 검증한 부분은 기존 테스트의 누락을 잘 보완하신 것 같아요. 최종 개수뿐만 아니라 입력값이 결과에 반영되었는지까지 확인해야 구매 로직을 제대로 검증할 수 있습니다. 👍

자동 번호 생성 책임을 LottoGenerator로 분리하신 것도 좋은 방향입니다. 다만 현재처럼 Lottos 내부에서 생성기를 직접 생성하면 테스트에서 자동 생성 결과를 제어하기 어려운 구조가 됩니다.

제가 말씀드린 테스트 방법은 LottoGenerator를 외부에서 주입받도록 하여, 테스트에서는 고정된 번호를 반환하는 생성기를 전달해 보는 방식입니다. 그러면 랜덤성에 의존하지 않고 다음과 같은 내용을 검증할 수 있어요.

  • 입력한 수동 로또가 결과에 포함되는지
  • 지정한 개수만큼 자동 로또가 생성되는지
  • 자동 생성된 번호가 실제 결과에 포함되는지
  • 수동 로또와 자동 로또를 합한 최종 개수가 맞는지

예를 들어 테스트에서는 항상 특정 번호를 반환하는 FakeLottoGenerator나 테스트용 람다를 주입할 수 있습니다.

다만 반드시 지금 당장 구조를 변경해야 한다는 의미는 아닙니다. 현재처럼 생성 책임을 LottoGenerator로 분리한 것만으로도 이전보다 테스트하기 좋은 구조가 되었고, 다음 단계로 생성기를 외부에서 주입받는 방식까지 고민해 보면 좋겠다는 의도였습니다. 😊

@DisplayName("당첨 번호가 정확히 6개면 생성된다")
void createsWinningNumbersWithSixNumbers() {
List<Integer> numbers = List.of(1, 2, 3, 4, 5, 6);
WinningNumbers winningNumbers = new WinningNumbers(numbers);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

현재 테스트는 WinningNumbers를 생성한 뒤 별도의 검증 없이 끝나고 있습니다.

생성 과정에서 예외가 발생하지 않는다는 것을 확인하려는 테스트라면 assertDoesNotThrow()로 의도를 명시할 수 있습니다. 또는 당첨 번호를 조회할 필요가 없다면, 잘못된 입력에서 예외가 발생하는 테스트만으로 생성 규칙을 충분히 검증할 수 있는지도 고민해 볼 수 있을 것 같아요.

테스트가 존재하는 것보다, 이 테스트가 어떤 실패를 잡아내려는지를 설명할 수 있는지가 더 중요하다고 생각합니다.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

해당 테스트는 6개가 아닌 경우에 대한 실패 케이스뿐 아니라 정확히 6개인 경우의 성공 케이스도 함께 확인한다면 당첨 번호는 6개여야 한다는 규칙의 양쪽 경계를 더 명확하게 표현할 수 있다고 생각하여 작성한 테스트였습니다.

다만 현재 테스트는 객체를 생성하기만 해서 그 의도가 코드에 드러나지 않았다는 점을 이해하였습니다. assertDoesNotThrow()를 사용하여 정상적인 6개의 번호로 생성될 수 있다는 의도를 명확히 하도록 수정하였습니다!

@johaeunn

johaeunn commented Aug 5, 2026

Copy link
Copy Markdown
Author

안녕하세요, 병웅님 좋은 리뷰 남겨주셔서 감사합니다!
남겨주신 코멘트를 하나씩 확인하며 수정해보았습니다. 😊

이번 리뷰에서는 특히 객체의 책임을 어디까지 둘 것인지와 객체가 생성된 순간부터 유효한 상태를 보장할 수 있는지를 많이 고민했습니다.

Application은 단순히 10라인을 맞추기 위해 메서드를 나누기보다는 말씀해주신 것처럼
로또 구매 → 당첨 정보 생성 → 통계 출력이라는 실행 흐름을 기준으로 분리하여 main()만 보더라도 프로그램의 전체 흐름을 파악할 수 있도록 수정하였습니다.

구조적으로는 Lotto가 담당하던 랜덤 번호 생성 책임을 LottoGenerator로 분리한 것과 당첨 번호와 보너스 번호를 하나의 당첨 정보인 WinningLotto로 묶은 부분이 가장 크게 변경되었습니다.

이외에도 LottoStatisticsOutputView의 역할을 다시 구분하고, 미당첨 결과를 null 대신 MISS로 표현하였으며, 테스트가 단순히 개수만 확인하는 것이 아니라 실제 동작을 충분히 검증하고 있는지도 다시 살펴보았습니다.

세부적인 고민과 수정 내용은 각 코멘트에 답변으로 남겨두었습니다. 🙇‍♀️

@quddaz quddaz left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

안녕하세요. 리뷰 반영해주셔서 감사합니다. 😊

이번 리뷰를 통해 단순히 메서드를 분리하는 데 그치지 않고, 로또 구매부터 당첨 정보 생성과 통계 출력까지 전체 흐름을 기준으로 객체의 책임을 고민하신 점이 좋았습니다.

특히 LottoGenerator로 랜덤 번호 생성 책임을 분리하고, WinningLotto를 통해 당첨 번호와 보너스 번호의 유효성을 생성 시점에 보장하도록 변경한 점이 인상적이었습니다. 또한 MISS를 도입할 때 null과의 차이 및 유지보수 비용을 비교하고, 수동 로또 번호가 실제 결과에 포함되는지 테스트를 보완하신 점에서도 깊이 고민하신 과정이 잘 드러났습니다.

남겨드린 코멘트를 바탕으로 설계 의도와 선택의 근거를 구체적으로 정리해주셔서 저도 즐겁게 리뷰할 수 있었습니다.
추가 코멘트를 남기겠습니다. 천천히 진행해주세요. 👍


Lottos lottos = new Lottos(manualLottoNumbers, autoLottoCount);

assertEquals(12, lottos.getLottos().size());

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

네, 맞습니다. 수동 로또 번호가 실제 구매 결과에 포함되는지 검증한 부분은 기존 테스트의 누락을 잘 보완하신 것 같아요. 최종 개수뿐만 아니라 입력값이 결과에 반영되었는지까지 확인해야 구매 로직을 제대로 검증할 수 있습니다. 👍

자동 번호 생성 책임을 LottoGenerator로 분리하신 것도 좋은 방향입니다. 다만 현재처럼 Lottos 내부에서 생성기를 직접 생성하면 테스트에서 자동 생성 결과를 제어하기 어려운 구조가 됩니다.

제가 말씀드린 테스트 방법은 LottoGenerator를 외부에서 주입받도록 하여, 테스트에서는 고정된 번호를 반환하는 생성기를 전달해 보는 방식입니다. 그러면 랜덤성에 의존하지 않고 다음과 같은 내용을 검증할 수 있어요.

  • 입력한 수동 로또가 결과에 포함되는지
  • 지정한 개수만큼 자동 로또가 생성되는지
  • 자동 생성된 번호가 실제 결과에 포함되는지
  • 수동 로또와 자동 로또를 합한 최종 개수가 맞는지

예를 들어 테스트에서는 항상 특정 번호를 반환하는 FakeLottoGenerator나 테스트용 람다를 주입할 수 있습니다.

다만 반드시 지금 당장 구조를 변경해야 한다는 의미는 아닙니다. 현재처럼 생성 책임을 LottoGenerator로 분리한 것만으로도 이전보다 테스트하기 좋은 구조가 되었고, 다음 단계로 생성기를 외부에서 주입받는 방식까지 고민해 보면 좋겠다는 의도였습니다. 😊

Comment thread src/main/java/domain/LottoRank.java Outdated
if (matchCount == 5) return THIRD;
if (matchCount == 4) return FOURTH;
if (matchCount == 3) return FIFTH;
return null;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

MISS를 추가한 방향은 자연스럽다고 생각합니다. 미당첨도 로또 결과 중 하나이므로 null보다 명시적인 상태로 표현할 수 있고, 호출하는 쪽의 방어 코드도 줄어들겠네요. 👍

다만 말씀하신 것처럼 MISS(0, 0)은 조금 고민해볼 필요가 있습니다. 미당첨은 0개뿐만 아니라 1개, 2개가 일치한 경우도 포함하기 때문에 matchCount = 0이 실제 미당첨 조건 전체를 표현하지는 못하기 때문입니다.

현재 matchCount가 등수를 판별하기 위한 기준으로만 사용되고, MISS의 실제 일치 개수를 조회하지 않는다면 MISS(0, 0)을 일종의 대표값으로 사용하는 것도 가능합니다. 다만 이 값이 “미당첨은 항상 0개 일치”라는 의미로 해석되지 않도록 주의해야 합니다.

반대로 matchCount가 통계나 출력에 사용된다면 MISS에 임의의 0을 넣기보다는, 당첨 등수 정보와 미당첨 상태를 분리하거나 matchCount를 별도로 관리하는 방법도 고민해볼 수 있습니다.

즉 현재 구조에서는 MISS(0, 0)을 사용해도 되지만, 0이 미당첨의 모든 경우를 의미하는 값은 아니라는 점을 명확히 하고 사용하는 것이 중요해 보입니다. 😊

Comment thread src/main/java/Application.java Outdated
Comment on lines +28 to +31
WinningNumbers winningNumbers = new WinningNumbers(inputView.readWinningNumbers());
LottoNumber bonusNumber = new LottoNumber(inputView.readBonusNumber());

winningNumbers.validateBonusNumber(bonusNumber);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

WinningLotto가 당첨 번호와 보너스 번호의 유효성을 보장하고, 한 장의 로또에 대한 등수를 판단하도록 한 방향은 자연스럽다고 생각합니다. WinningLotto가 당첨 정보와 등수 판단에 필요한 규칙을 함께 알고 있기 때문입니다. 👍

다만 WinningLotto가 직접 일치 개수를 계산하는 것까지 반드시 담당해야 하는지는 한 번 더 고민해보면 좋겠습니다.

“두 로또의 번호가 몇 개 일치하는가”는 Lotto끼리의 비교에 가까운 책임이므로, 예를 들어 Lotto.countMatchingNumbers(Lotto other)처럼 Lotto에 둘 수 있습니다. 이후 WinningLotto는 해당 결과와 보너스 번호를 바탕으로 등수를 판단하는 역할을 맡을 수 있습니다.

정리하면 다음과 같이 나눌 수 있을 것 같아요.

  • Lotto: 다른 로또와 일치하는 번호 개수 계산
  • WinningLotto: 당첨 번호·보너스 번호를 포함한 당첨 판정
  • LottoStatistics: 판정된 등수 집계

현재처럼 determineRank()WinningLotto에 둔 것은 적절해 보이며, 내부의 일치 개수 계산 책임까지 어디에 둘지는 객체 간 데이터 노출과 응집도를 기준으로 추가로 고민해보면 좋겠습니다. 😊

Comment on lines +20 to +22
public List<Lotto> getLottos() {
return List.copyOf(lottos);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

방어적 복사를 통한 안전한 정보 전달 좋습니다👍

Comment thread src/main/java/domain/WinningLotto.java Outdated
Comment on lines +21 to +23
private boolean isBonusNumberMatched(Lotto lotto) {
return lotto.getNumbers().contains(bonusNumber);
}

@quddaz quddaz Aug 5, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

해당 부분도 고민해보시면 좋을 것 같아요.

이미 Lotto 객체를 전달받고 있으므로, 내부 번호를 직접 꺼내기 위해 getter를 사용하는 대신 Lotto에 일치 여부를 판단하는 메서드를 제공하고 boolean 결과만 받아오는 방식도 생각해볼 수 있습니다. 이렇게 하면 번호 데이터를 외부에 노출하지 않고도 필요한 비교를 수행할 수 있어요. 😊

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

말씀해주신 것처럼 getNumbers()로 내부 번호를 꺼내 직접 비교하지 않고 Lotto 번호 포함 여부를 직접 판단하도록 containsNumber() 메서드를 추가하였습니다!

return winningNumbers.contains(number);
}

int countMatchingNumbers(Lotto lotto) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

접근 제어자 지정해주세요!

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

해당 메서드는 WinningNumbersLotto로 통합하면서 Lotto의 메서드로 이동하였고, 접근 제어자도 명시하도록 수정하였습니다!

@quddaz quddaz Aug 5, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Application의 책임을 정리한 점이 좋습니다.

여기서 한 단계 더 나아가고 싶다면, 로또 시스템의 실행 흐름을 담당하는 객체를 별도로 만들어보는 방법도 있습니다. Application은 엄밀히 말하면 프로그램의 진입점이자 실행기 역할에 가깝기 때문에, 로또 구매부터 당첨 정보 생성과 통계 계산까지의 흐름을 LottoGame, LottoController과 같은 객체에 위임할 수 있습니다.

이렇게 하면 해당 실행기가 사용할 LottoGenerator도 외부에서 주입할 수 있어, 테스트에서 자동 생성 결과를 원하는 값으로 제어하기 쉬워집니다.

현재 구조로도 전체 흐름은 충분히 읽히지만, 실행 흐름을 별도 객체에 위임하는 구조도 생각해볼 수 있습니다. 다만 현재 규모에서 객체를 추가하는 비용보다 얻는 이점이 큰지 함께 비교해보면 좋겠습니다.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

실행 흐름을 별도 객체로 분리하면 Application 을 진입점 역할에 집중시킬 수 있고, LottoGenerator 를 외부에서 주입하여 실행 흐름까지 테스트하기 쉬워진다는 장점이 있다는 것을 이해하였습니다.

다만 현재는 로또 구매, 당첨 정보 생성, 통계 출력 정도로 실행 단계가 비교적 단순하고, 실제 계산이나 규칙은 각각의 도메인 객체가 담당하고 있어 Application은 주로 객체를 생성하고 연결하는 역할을 하고 있습니다. 이 상태에서 LottoGame이나 LottoController를 추가하면 현재로서는 기존 흐름을 한 번 더 감싸는 역할이 대부분일 것 같다는 생각이 들었습니다.

따라서 지금은 현재 구조를 유지하되, 이후 실행 흐름이 더 복잡해지거나 LottoGenerator 를 주입하여 전체 흐름을 제어하는 테스트가 필요해진다면 별도의 실행 객체를 분리하는 방향을 고려해보겠습니다. 감사합니다!

@boorownie
boorownie merged commit 0d33417 into next-step:johaeunn Aug 8, 2026
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.

3 participants