Skip to content

[그리디] 이태규 로또 미션 1,2단계 제출합니다.#156

Open
Cappucciyes wants to merge 30 commits intonext-step:cappucciyesfrom
Cappucciyes:week4
Open

[그리디] 이태규 로또 미션 1,2단계 제출합니다.#156
Cappucciyes wants to merge 30 commits intonext-step:cappucciyesfrom
Cappucciyes:week4

Conversation

@Cappucciyes
Copy link
Copy Markdown

@Cappucciyes Cappucciyes commented Mar 30, 2026

🙋‍♂️ 인사

안녕하세요, 그리디 4기 백엔드 이태규입니다. 시간을 내주어 코드 리뷰 및 질의응답을 해주셔서 감사합니다.


🚗 단계별 설명

1단계 - <로또 자동 구매>

구입 금액을 입력받아 형식을 검증하고, 해당 금액에 비례하는 수량만큼 로또 번호를 자동 생성합니다.

2단계 - <로또 당첨>

지난주 당첨 번호를 입력받고, 발행된 로또 번호와 대조하여 당첨 통계(일치 개수 및 상금)와 총 수익률을 계산하여 출력합니다.


🤔 고민한 내용

  • Magic Number 관리
    • 로또라는 도메인 특성상 (숫자 범위 1~45, 숫자 개수 등) 여러 Magic Number가 자연스럽게 코드 전반에 등장하게 되었습니다. 특히 개발을 진행하면서, 값들이 서로 연관되는 경우 (예: 일치 개수에 따른 당첨 금액) 또는 테스트 코드에서도 해당 값들이 중요한 기준으로 사용되는 경우가 많아지면서, 이러한 Magic Number들을 어떻게 관리하는 것이 좋을지 고민하게 되었습니다.
  • 일급 컬랙션에서의 데이터를 반환하는 역할
    • 이번 미션에서는 한 뭉치의 데이터을 활용하는 시스템인데, 이때 일급 컬랙션에게 데이터 가공의 역할을 주는게 자연스러울까라는 의문이 들어요
  • 객체를 대량으로 생산 하는 경우 어떻게 하면 좋을까?
    • 이번 미션에서 로또를 주어진 값에 따라 여러 개의 인스턴스를 만드는 역할을 누군가는 했어야 하는데, 이 역할을 컬랙션이 하는 것이 자연스러운가에 의문이 들었어요.
    • 저는 컨트롤러의 역할은 오로지 주어진 값에 따라 여러 개의 인스턴스를 만드는 것 (해당 인스턴스를 반환하는 메소드를 반환하는 함수 호출)이 더 코드가 직관적일 것이라고 생각 했어요. 앞서 말한 메소드에 심하게 의존하는게 없지는 않지만, 로또의 생성 규칙이 어떻게 바뀌든 상관없이 해당 컨트롤러의 작동 방식이 다르지 않으므로 이러한 방향으로으 코드를 작성하기로 결론을 내렸습니다.

❓ 질문사항

  1. Magic Number 관련해서 자명하더라도 원시값은 변수를 활용해서 관리하는 것이 더 좋을 것 같다는 생각은 들지만, 이렇게 특정 항황에서 쓰이는 Magic Number가 많아질수록, 특히 Magic Number등 끼리 약하게나마 서로 연관성이 있으면 어떻게 관리하는 것이 좋을까요? 일단 저는 넓은 범위에서 쓰일 것 같은 것들은 하나의 클래스로 묶어 한번에 관리하는 방식으로 가긴 했는데, 이 마저도 시스템이 점점 복잡해지면 잘 안될 것 같다는 생각이 드네요...

  2. LottoFactory를 작성하여 필요한 만큼 Lotto를 생산할 수 있도록 구현을 해보았습니다. 편의상 Lotto에서 Lotto를 생성하는 역할을 하나의 클래스로 분리를 한 것이긴 하지만, 테스트 하기고 좋고, 제 역할을 잘 하는 것 같아 나름 괜찮아 보이는데, 혹시 이런 방식으로 구현을 하면 하나의 책임(객체 생성)이 두 곳에 존재하는 구조여서 좋은 코드인지 판단이 안서는 것 같아요. 리뷰어님이라면 이 상황에서 해당 코드는 유지보수하기 좋은 코드처럼 느껴지나요?

  3. 이거는 조금 다른 느낌의 질문이긴 한데, 제가 미션을 수행하면서 하나의 기능을 구현하는 과정에서 여러 관련 기능을 동시에 개발하게 되는 경우가 많았어요. 해당 기능을 구현하려면 여러가지 필요한 기능들도 같이 덩달아 구현하게 되더라고요. 당연히 그렇게 생기는 큰 뭉텅이가 한 번에 저장되는 커밋들 때문에 버전 컨트롤이 잘 안되는 것 같네요... 혹시 리뷰어님은 어떤 기능을 구현하려할 때 어떤 방식으로 접근을 하시나요? 리뷰어님이 어떤 방식으로 기능 구현을 접근하는지 궁금합니다.

@Cappucciyes Cappucciyes changed the title Week4 [그리디] 이태규 로또 미션 1,2단계 제출합니다. Mar 30, 2026
Copy link
Copy Markdown

@haeyoon1 haeyoon1 left a comment

Choose a reason for hiding this comment

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

안녕하세요 태규님!
이번 미션도 수고 많으셨어요~ 코드에 고민한 흔적이 많이 보이네요👍👏

PR에 고민한 내용 또한 구체적으로 남겨주셔서 리뷰 남기기 수월했습니다!
이해 안되는 부분이나 질문이 있다면 편하게 말해주세용 잘 부탁드립니다🙌

✏️ 코드 스타일에 관한 피드백

현재는 메서드 순서가 조금 섞여있는 것 같아요!
https://dodop-blog.tistory.com/277
해당 블로그를 보고 메서드 순서를 통일해주시면 조금 더 읽기 쉬운 코드가 될 것 같습니다! 특히

메소드 : 접근자 기준으로 작성하지 않고, 기능 및 역할별로 분류하여 기능을 구현하는 그룹별로 작성이 이루어지도록 해야한다. (public 사이에 private 메소드가 존재할 수 있다)

요 부분을 신경써주시면 좋을 것 같아요. 현재는

B메서드(){
   B메서드 호출
}
A메서드(){
   B메서드 호출
}

이렇게 작성된 부분이 조금 있는데 코드의 흐름을 자연스럽게 하기위해서는

A메서드(){
   B메서드 호출
}
B메서드(){
   B메서드 호출
}

이렇게 수정해주시면 좋을 것 같아요!

🤔 QnA

1. 매직넘버 관리

Magic Number등 끼리 약하게나마 서로 연관성이 있으면 어떻게 관리하는 것이 좋을까요?

연관이 있다는건 무슨의미일까요?!

일단 매직넘버 관리에대해 현재는 LottoSettingsConstants라는 클래스에 변수들을 모아두셨더라구요! 이렇게 하나의 클래스 내부에 모아두게되면, 변수를 모든 클래스에서 재사용하기 쉬워, 상수 조건이 바뀌었을 때 이곳저곳 수정할 필요 없이 하나의 클래스만 확인 및 수정하면 된다는 편리함이 있습니다.

다만 저는 개인적으로 그냥 각 클래스 내부에서 한눈에 확인하는 것이 편하다 생각해, 각각의 클래스 상단에 해당 클래스에 필요한 상수들을 final static 선언해두는 방법을 더 선호하긴합니다!
이 부분은 취향차이라서 이대로 진행하셔도 좋을 것 같아요!

2. Lotto와 LottoFactory의 분리

저는 작성하신 구조가 아주 좋아보입니다! 로또 생성의 책임을 두 군데에서 담당한다기보다는, LottoFactory에서 생성 책임을 담당하고, Lotto는 생성된 값을 표현하는 역할에 집중하여 적당한 책임분리가 이루어진 것 같아요👍

3. 도메인 설계 과정

저는 먼저 Readme에 기능 목록을 쭉 작성한 뒤 플로우대로 차근차근 하나씩 기능을 구현하는 편이에요.
태규님은 Readme의 기능 요구사항에 간단한 요구사항만 적어주셨지만, 더 구체화 시켜보면 어떨까요?

- [ ] 로또 구입 금액을 입력한다.
	- [ ] 로또 금액은 ~~ 이어야한다. (조건)
- [ ] 로또 ~~ 를 출력한다.
- [ ] 지난 주 당첨 번호를 입력한다.
  - [ ] (조건)
- [ ] 당첨 통계를 출력한다.
  - [ ] (조건1)
  - [ ] (조건2) ...

위와 같이 태규님께서 이번 미션을 진행하며 고려할 조건 및 흐름을 작게 잘라 구체적으로 적어놓은 뒤 구현하면, 여러 기능을 한번에 구현할 일은 없을 것 같아요!

(new!) 4. controller 분리

현재 태규님은 controller를 LottoPurchaseController와 LottoResultCalculatorController로 나누어 작성해주셨어요. 로또를 구매하는 로직과 계산하는 단계를 분리함으로써 단계적으로 코드를 이해할 수 있다는 장점이 있는 것 같아요!

하지만 Main 클래스를 보면, 같은 lottoBatch를 인자로 받아 사용하고 있는데, 결국에는 같은 하나의 객체를 두고 절차만 두개의 클래스로 나누어 작성한 것 처럼 보일 수 있을 것 같아요. 따라서 하나의 컨트롤러에 구매 / 계산 로직을 2개의 메서드로 나누어 구현하는 방법도 있을 것 같네요!

해당 부분은 수정이 필요하다는 것은 아니고 컨트롤러를 나누었을 때의 이점에대해 한번 더 고민해보셨으면해서 남겨보았습니다!

이번 미션 파이팅입니다!🔥


import static java.lang.Math.min;

public class LottoNumberGenerator implements NumberGenerator {
Copy link
Copy Markdown

@haeyoon1 haeyoon1 Mar 31, 2026

Choose a reason for hiding this comment

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

👍
인터페이스를 implement 한 이유가 무엇일까요? (인터페이스의 장점)

바로 랜덤 상수를 만드는 클래스를 만드는 방법도 있을텐데, 인터페이스의 구현체로 해당 클래스를 만드신 이유가 궁금합니다!

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.

랜덤성이 있는 부분은 최대한 코드에거 고립시켜 관리해야한다고 생각하기 때문입니다. 랜덤성이 있는 부분은 최대한 그 부분만 다른 방식으로 대체 될 수 있도록 구현하는 것이 보다 유지보수하기 수월한 코드가 될 것 같아 인터페이스로 구현하게 되었어요.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

태규님께서 정의하신 LottoBatch의 역할은 무엇인가요?

Readme에는 구매한 여러 개의 로또들을 관리하는 일급 컬렉션이라고 정의해주셨는데, 당첨 결과까지 계산하고있어 조금 많은 책임을 가지고있는 것 같아요 🥲

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.

일급 컬랙션의 역할은 로또들을 관리하면서, 로또가 여러개 있을 때 유의미한 계산 (평균, 최댓값 등)을 계산하는 것이라고 생각을 했습니다.

하지만 코드를 읽어보니 당첨 결과를 계산하는 것은 외부의 입력값(당첨번호)의 개입이 필요하니, 이 계산은 컨트롤러가 하는 것이 더 좋겠다는 생각이 들어 수정하였습니다!.

LottoSettingsConstants.FIVE_MATCH_PRICE,
LottoSettingsConstants.SIX_MATCH_PRICE
);
List<Lotto> lottos;
Copy link
Copy Markdown

@haeyoon1 haeyoon1 Mar 31, 2026

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.

헉, 이런걸 놓쳤네요.... 다음부턴 사소한 실수가 없도록 더 노력할게요...

접근제어자는 외부로 부터 원치 않은 접근을 통해 해당 상태가 오염되거나 원치 않은 메소드 호출을 막아주기에 꼭 필요하다고 봅니다. 또한 코드를 읽는 입장으로서, 이 코드가 어디에 사용될 수 있는지에 대한 의도도 대략적으로 주기에 가능하면 꼭 있으면 좋겠다라는 생각이 드네요.

Comment on lines +33 to +46
public void purchase() {
int userCashInput = inputView.getUserCashInput();
checkPriceHigherThanSingleLottoPrice(userCashInput);

int lottoCount = userCashInput/ LottoSettingsConstants.LOTTO_PRICE;
for (int i = 0; i < lottoCount; i++) {
getLotto();
}

List<LottoDto> lottoDtos = lottoBatch.getAllLotto().stream()
.map(this::wrapLottoIntoDto).toList();

outputView.printPurchaseResult(lottoDtos);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

태규님이 생각하시는 controller의 역할은 무엇인가요?

저는 해당 컨트롤러가 도메인의 역할도 하고있는 것 같아요!

  • checkPriceHigherThanSingleLottoPrice (입력 받은 금액에 대한 검증)
  • int lottoCount = userCashInput/ LottoSettingsConstants.LOTTO_PRICE; (구매 개수 계산)
  • for (int i = 0; i < lottoCount; i++) 로또 반복 생성
  • dto 변환

너무 많은 책임을 가지고 있는 것 같은데, 도메인 각 기능에 대해 해당 기능은 어느 도메인 혹은 계층이 가져야할지 고민해보시면 좋을 것 같아요!

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.

저는 컨트롤러가 두 가지 역할을 해야 한다고 생각합니다
- 필요한 함수들을 호출하는 중재자 역할 및 일부 비즈니스 로직 실행
- 입력/출력에 대한 적절한 데이터 정제입니다.
그래서 입력값 검증, 받은 금액만큼 로또를 반복 생성하는 로직, 그리고 DTO로 변환하는 과정은 컨트롤러가 담당하는 것이 맞다고 보았습니다.

다만, 현재처럼 하나의 메소드가 그 모든 책임을 가지는 것은 문제가 있다고 생각했고, 이에 따라 로또를 반복 생성하는 부분은 별도의 메소드로 분리하여 책임을 나누었습니다.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

image

태규님이 작성해주신 컨트롤러의 역할에 대해 잘 읽어봤습니다!

  • 필요한 함수들을 호출하는 중재자 역할 및 일부 비즈니스 로직 실행

컨트롤러는 어플리케이션의 흐름을 관리하는 역할이기에 요 의견에 동의합니다.
(다만 비즈니스 로직 실행이라는 말이 조금 모호한 것 같은데, 비즈니스 로직 실행 == model 의 메서드를 적절히 불러온다 라는 의미라면 맞는 것 같고, 만일 비즈니스 로직을 직접 작성하는 것이라면 model 계층의 역할과 섞인 것 같아요)

  • 입력/출력에 대한 적절한 데이터 정제입니다.

하지만 입력/출력 로직의 호출이 아닌 데이터 정제 부분에 대해서는 저는 조금 다른 의견을 가지고있어요!
컨트롤러에서는 도메인과, view의 흐름만 통제해야지, 구체적인 비즈니스 로직에 대해서는 알고있으면 안된다고 생각해요.

그전에 model 계층의 역할에 대해 한번 정의하고 넘어가보겠습니다! 찾아보니 "model은 애플리케이션의 핵심 데이터와 비즈니스 로직을 나타낸다."라고 하네요.


🤔💭 만일 로또의 가격이 1000원에서 500원으로 바뀌었다거나, 로또 가격 검증 조건이 추가 및 수정 되었다고 가정해보겠습니다!

현재 코드에서는 controller 계층이 수정되겠네요! 하지만 mvc 패턴에서 각 계층의 정의에의하면 비즈니스 로직을 담고 있는 것은 model 계층이기때문에, 저는 model 부분 코드를 먼저 읽어볼 것 같아요.

저는 이러한 이유로 검증로직을 도메인 부분으로 넘겨야한다고 생각하는데, 태규님은 요 의견에대해 어떻게 생각하시나요?
만일 옮겨야하는 적절한 도메인 클래스가 없다면, 해당 값은 단순한 원시타입의 숫자나 값이 아닌 하나의 역할을 하는 객체로 포장이 필요했던것은 아닐까요?!

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.

컨트롤러에서는 도메인과, view의 흐름만 통제해야지, 구체적인 비즈니스 로직에 대해서는 알고있으면 안된다고 생각해요.

이 부분이 저에게 컨트롤러가 어떤 역할을 해야하는지 잘 정리해준 것 같습니다... 확실히 특정 비즈니스 로직이 컨트롤러에 있는 설계는 것 자체가 객체지향적이지 않은 것 같네요. 해당 부분을 참고하여 LottoResultCalculatorController를 수정해 보았습니다.

Comment on lines +48 to +51
protected void getLotto() {
Lotto lotto = this.lottoFactory.generateLotto();
this.lottoBatch.add(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.

getLotto는 일반적으로 Lotto 정보를 반환할 때 사용하는데, 해당 메서드에서는 lotto를 추가하고있네요!
메서드 명 수정이 필요해보입니다~

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.

수정했습니다!

}

public List<Integer> getMatchCountPerLotto(List<Integer> winningNumbers) {
this.checkIfNumbersAreValid(winningNumbers);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

getReturnRatio에서 checkIfNumbersAreValid 검증 된 채로 checkIfNumbersAreValid에 들어오게 되는데, 위 검증 로직은 중복이된 것 같아요!

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.

검증에 대한 역할을 고민하면서 수정했습니다! 이제 검증의 책임은 Lotto 객체에서 처리하는 것으로 바꿨습니다.

Comment on lines +59 to +66
private int countMatches(List<Integer> lottoNumber, List<Integer> winningNumbers) {
this.checkIfNumbersAreValid(winningNumbers);
Set<Integer> lottoNumberSet = new HashSet<>(lottoNumber);
Set<Integer> winningNumberSet = new HashSet<>(winningNumbers);
lottoNumberSet.retainAll(winningNumberSet);

return lottoNumberSet.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.

getMatchCountPerLotto에서 lotto.getNumbers()로 Lotto 필드값을 직접 꺼내 정답 로또와 몇개 일치하는지를 비교하고있네요!

  1. 일단 객체 외부에서 필드값을 꺼내 로직을 수행하는 것은 위험해요! 비슷한 이유로 getter를 지양하라고하는데, 그 이유가 무엇일까요?
  2. 해당 로직은 LottoBatch가 아닌 다른 클래스로 옮길 수 있을 것 같아요. 어디로 옮기면 객체가 스스로의 책임을 다할 수 있을까요?

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. 제가 이해한 바로는 getter를 사용하여 필드값을 꺼내면, 특히 해당필드가 원시값이 아닌경우, 필드가 외부의 영향에 노출되기 때문이라고 알고 있습니다. 하지만 이 문제는 해당 필드를 deep copy 하면 될 것 같아 큰 문제는 아닌것 같아요. 그리도 어떤 자료들은 "getter에 의존하는 코드가 많아져 독립성을 해친다"라고 설명읗 하는데, 이는 getter를 지양하는 것 보단 getter를 왠만하면 쓰지 말자라고 하는 것이 더 맞는 방향인 것 같아요.

  2. 조금 고민해 봤는데, 이를 ENUM에서 해결할지, Lotto 에서 해결할지 고민했습니다. 일단 도메인의 조건을 확인한다는 측면에서 Lotto에 책임을 전가하자는 쪽으로 마음이 기울었는데, 이는 해윤님은 어떻게 생각하는지 궁금해요.

    • ENUM을 계산하는 것은 ENUM에서 처리하는 것이 더 자연스러울까요?
      • 혹시나 해서 제가 ENUM을 ENUM내부에서 계산하는 방식을 주석처리 했습니다. 시간이 되면 그 부분도 한 번 살펴주시고, 해당 구현 방법은 어떤지 의견을 공유해주시면 감사하겠습니다.
    • 아니면 Lotto에서 필요한 ENUM을 계산하는 것이 더 적절한 방식일까요?

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. 제가 이해한 바로는 getter를 사용하여 필드값을 꺼내면, 특히 해당필드가 원시값이 아닌경우, 필드가 외부의 영향에 노출되기 때문이라고 알고 있습니다.

👍👍👍

  1. 조금 고민해 봤는데, 이를 ENUM에서 해결할지, Lotto 에서 해결할지 고민했습니다. 일단 도메인의 조건을 확인한다는 측면에서 Lotto에 책임을 전가하자는 쪽으로 마음이 기울었는데, 이는 해윤님은 어떻게 생각하는지 궁금해요.

저도 단순히 두 로또를 비교해 몇 개 일치하는지를 반환하는 메서드라면 Lotto에서 진행하는 것이 맞다고 생각합니다!

하지만 말씀하시대로 맞춘 개수를 기반으로 Enum을 반환하는 것은 또 Enum의 책임이라는 생각이 들어요.

그렇다면 해당 메서드가 너무 많은 역할을 하기때문에 이런 고민이 드셨을 수 도 있어요!
로직을 조금 나누어서

  1. 로또 개수 비교
  2. 일치 개수에따른 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.

이를 통해 역할 분배가 애매하면 차라리 역할을 조금 더 쪼개 보는 것도 나쁘지 않은 방법이라는 것을 알게 되었습니다. 참고하고 수정했습니다!

double earnResult = 0.0;
for (int i = 0; i < WINNING_COUNT.size(); i++) {
int currentCount = WINNING_COUNT.get(i);
int totalCount = result.stream().filter(matchCount -> currentCount == matchCount).toList().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.

long totalCount = result.stream()
                .filter(matchCount -> currentCount == matchCount)
                .count();

리스트의 요소 개수 만을 세는 것이라면, count()를 사용해 의도를 명확하게 할 수 있을 것 같아요!

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.

앗 몰랐던 stream 함수네요.... 참고하겠습니다.

Comment on lines +12 to +18
private final List<Integer> WINNING_COUNT = List.of(3,4,5,6);
private final List<Integer> WINNING_PRICE = List.of(
LottoSettingsConstants.THREE_MATCH_PRICE,
LottoSettingsConstants.FOUR_MATCH_PRICE,
LottoSettingsConstants.FIVE_MATCH_PRICE,
LottoSettingsConstants.SIX_MATCH_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.

현재 WINNING_COUNT와 WINNING_PRICE를 각각 변수로 생성한 뒤 아래에서 인덱스를 사용해 연결시키고 있는 것으로 보여요! 인덱스를 사용해 매칭을 하게되면 실수할 확률이 높아질 것 같아요!
연관있는 두 상수를 묶어 사용할 수 있는 방법이 있습니다.

  1. Map을 사용하여 묶기
  2. 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.

코드 리뷰를 보고 ENUM에 대해 공부했습니다. 자바 ENUM 엄청 강력하네요

Comment on lines +69 to +72
this.checkIfDuplicateExist(numbers);
this.checkIfNotEnoughNumbers(numbers);
this.checkIfTooManyNumbers(numbers);
this.checkIfNumbersAreInRange(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.

(참고) this는 생략 가능해요!

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.

this는 의도적으로 계속 쓰고 있습니다. 아무래도 이 변수는 이 스코프에서만 쓰이는 변수인가 아니면 클래스의 필드인가 가끔씩 햇갈리더라고요....

혹시 this를 너무 많이 남발하면 코드가 덜 깔끔해 보일까요...? 혜윤님은 어떻게 생각하시나요..?

Copy link
Copy Markdown

@haeyoon1 haeyoon1 Apr 2, 2026

Choose a reason for hiding this comment

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

생성자에서는 필수적이긴한데, 저는 그냥 그 외의 부분에는 안쓰고있어요😅 깔끔한게 좋아서 ....
근데 헷갈리신다면 필드에 대해서는 일단 하던대로 사용하시는게 좋을 것 같지만, 메서드에서는 보통 사용하지않는 편입니다!

@Cappucciyes
Copy link
Copy Markdown
Author

다음과 관련해서 몇 가지 궁금한 점이 있습니다.

  • Enum 사용 관련

    • 개발 편의상 실제로는 필요 없을 수도 있는 값을 enum에 포함시켰는데 (LottoResultZERO, ONE, TWO), 이런 선택이 괜찮은지 궁금합니다.
    • 확장성 측면에서는 도움이 된다고 생각했는데, 다르게 생각하면 안쓰이는 코드를 적게 되는거여서 해윤님의 생각은 어떤지 궁금합니다.
  • 검증 책임에 대해

    • 현재는 입력값(예: 당첨 번호)에 대한 검증을 일급 컬렉션이 담당하도록 구현했는데, 이런 구조가 자연스러운지 고민이 됩니다.
    • 별도의 검증 클래스를 두는 방식도 고려했는데, 이 경우 “검증을 통과한 값은 항상 안전하다”는 전제 하에 코드를 작성해도 괜찮은지 궁금합니다.
    • 일반적으로 검증 책임은 어느 계층/객체가 가지는 것이 좋은지 의견을 듣고 싶습니다.
  • Magic Number 관련

    • 처음 Magic Number에 대해 질문했을 때, 일치 개수에 따른 당첨 금액, 일치 개수처럼 의미적으로 연관된 경우를 말하는 거였어요, 이러한 Magic Number가 많아지면 서로 의존되어 관리가 어려워질 것 같은데, 이와 관련해서 따로 Magic Number에 대한 관리에 대해 질문을 하고 싶었습니다.
    • 현재는 “값이 틀리지 않을 것”이라는 가정 하에 코드를 작성했는데, 이런 접근이 위험한지 고민됩니다.
      • 예를 들어 가격이 음수로 잘못 설정되는 경우까지도 방어적으로 검증하는 것이 좋은지 궁금합니다.
  • 테스트 코드에서의 Magic Number

    • 테스트를 작성하면서도 Magic Number를 그대로 사용하는 경우가 많았는데, 이 부분도 개선이 필요한지 궁금합니다.
    • 반대로, 모든 값을 추상화하려고 하면 테스트 가독성이 떨어지는 것 같아 적절한 균형을 어떻게 잡는 것이 좋은지 궁금합니다.
  • Mock 객체 관리

    • 통합 테스트를 작성하면서 다양한 Mock 클래스를 만들게 되었는데, 이를 어떻게 구조적으로 관리하는 것이 좋은지 궁금합니다.
    • 예를 들어
      • Mock하는 클래스의 페키지 위치 기준으로 패키지를 나누는지
      • 혹은 실제로 사용되는 도메인/패키지 기준으로 배치하는지
      • 일반적인 기준이나 추천 방식이 있다면 알고 싶습니다.

@haeyoon1
Copy link
Copy Markdown

haeyoon1 commented Apr 2, 2026

리뷰를 모두 잘 반영해주네요!👍 몇 가지 새로운 리뷰와 답변 남겨두었으니 확인부탁드려요~

추가로 주석 처리된 메서드나, 빈 테스트문 등은 삭제해주세요!

Enum 사용 관련
개발 편의상 실제로는 필요 없을 수도 있는 값을 enum에 포함시켰는데 (LottoResult의 ZERO, ONE, TWO), 이런 선택이 괜찮은지 궁금합니다.

Zero, One, Two를 삭제하고 일치개수가 3개인 경우부터 저장하는 방법도 있지만, 요 부분은 원하시는대로 하셔도 좋을 것 같아요!
실제로 로또 일치개수가 0개, 1개, 2개인 경우가 없는 것 도 아니고, 출력만 3개 이상 일치한 경우부터 하는 것이기에 이정도는 완전 불필요한 코드라고 볼 수는 없을 것 같습니다!

다만 이렇게 될 경우 현재는 outputview에 출력해야할 값들을
List<LottoResult> WINNING_RESULT = List.of(LottoResult.THREE, LottoResult.FOUR, LottoResult.FIVE, LottoResult.SIX);
이렇게 하드코딩 해 넣어주고있는데요, 도메인의 책임을 view가 조금 알게된다는 단점이 있을 것 같아요. 이 부분은 해당 로직을 다른 부분으로 옮김으로써 해결할 수 잇을 것 같습니다~

검증 책임에 대해
현재는 입력값(예: 당첨 번호)에 대한 검증을 일급 컬렉션이 담당하도록 구현했는데, 이런 구조가 자연스러운지 고민이 됩니다.
별도의 검증 클래스를 두는 방식도 고려했는데, 이 경우 “검증을 통과한 값은 항상 안전하다”는 전제 하에 코드를 작성해도 괜찮은지 궁금합니다.
일반적으로 검증 책임은 어느 계층/객체가 가지는 것이 좋은지 의견을 듣고 싶습니다.

검증의 책임은 객체에 두는 것이 적절하다고 생각해요!
위 리뷰에도 일부 작성했지만, 저희가 로또를 List이 아닌 Lotto라는 객체에 담아 가지고있잖아요? 원시값으로 값을 들고 있는 것이 아닌 객체에 담아 가지고 다니는데에는, 해당 값이 단순 문자의 list가 아닌 의미, 규칙을 가진 하나의 객체로써 들고다닐 만큼의 중요함이 있기때문이라 생각합니다.

또한 해당 객체는 객체 내부에 정의된 규칙을 만족한 믿을 수 있는 객체구나~ 라고 믿고 사용하는 것은 당연한 사고방식인 것 같아요!

하지만 현재는 검증의 책임이 일부 컨트롤러에도 작성되어있던데 이부분은 수정이 필요해보입니다!

Magic Number 관련
처음 Magic Number에 대해 질문했을 때, 일치 개수에 따른 당첨 금액, 일치 개수처럼 의미적으로 연관된 경우를 말하는 거였어요, 이러한 Magic Number가 많아지면 서로 의존되어 관리가 어려워질 것 같은데, 이와 관련해서 따로 Magic Number에 대한 관리에 대해 질문을 하고 싶었습니다.

이런 의미에서의 연관성이라면 사실 어쩔수없는 문제인 것 같아요😅 일치 개수에 따라 당첨 금액이 달라지는 것은 당연한 부분이기도하고, 그것까지가 비즈니스 요구사항이기때문에 해당부분을 개선할 방법이 딱히 떠오르지는 않네요🤔

매직넘버에대한 검증이라는 것은 public static final int LOTTO_MINIMUM_NUMBER = 1;를 입력하는 과정에서
public static final int LOTTO_MINIMUM_NUMBER = 3;처럼 잘못 입력한 경우에대한 검증을 어떻게 진행할지에대한 말일까요?

상수화한 상수에 대한 값을 믿고 사용하는 것은 괜찮은 것 같아요.

상수화를 하는 이유가 무엇일까요? 저는 이곳 저곳에서 반복적으로 숫자로 값을 하드코딩해 사용하면 어떤 곳에서는 실수로 다른 값을 입력할 수 있기때문도 있다고 생각해요.
그런 의미에서 단순 숫자들을 변수로 바꾸고 변수를 재사용 해 사용하는 것은 그 실수를 줄여주는 방법이라고 생각합니다! 따라서 의미를 가진 모든 상수는 모두 상수화를 진행한 뒤 믿고 사용하는 것이 좋을 것 같아요!
(제가 이해한 바가 맞을까요??)

Mock 객체 관리
통합 테스트를 작성하면서 다양한 Mock 클래스를 만들게 되었는데, 이를 어떻게 구조적으로 관리하는 것이 좋은지 궁금합니다.

해당 질문에 답변을 드리기전에 태규님은 어떤 경우에 Mock객체를 만드셨나요? Mock객체란 왜 사용하는 걸까요?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

dto를 사용하는 이유는 무엇인가요?

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.

dto는 모델 외 다른 부분들이 모델의 구현과 별개로 필요한 정보를 전달 받을 수 있도록 해줄 수 있는 포장지 같은 클래스로 이해하고 있습니다. 예를 들어 OutputView' 의 printStats`은 모델이 어떤 방식으로 로또의 결과를 계산하는지 모르지만, 결과를 Map 형식으로 받아와 결과를 출력합니다.

지금은 모델이 단순해 전달해야하는 정보가 적지만, 더 복잡한 모델을 가진 시스템에서는 뷰가 필요한 정보만을 컨트롤러를 통해 전달 받을 수 있게 해주어 편리하기에 확장성을 고려하여 dto를 적용해봤어요.

Comment on lines +12 to +13
private final List<Integer> numbers;
private int index = 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.

LottoNumberGenerator는 단순히 랜덤값을 생성하는 util 클래스처럼 보입니다!
해당 클래스가 필드값을 가질 필요가 있을까요??

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.

이는 LottoNumberGenerator의 구현 방식에 의해 필요하다고 봅니다!

저는 어떻게 하면 중복되지 않는 숫자를 반환할까 고민해봤는데, 가장 확실한 방법은 1부터 45까지 있는 리스트를 무작위로 섞은 뒤, 처음 몇개를 뽑는 방식으로 구현하였습니다!

해당 구현체는 도메인(로또)를 생각하고 구현되었고, 오로지 도메인에서만 활용되므로 도메인 페키지로 옮겼습니다!

LottoBatch lottoBatch = new LottoBatch(new ArrayList<Lotto>());
NumberGenerator numberGenerator = new LottoNumberGenerator();
LottoFactory lottoFactory = new LottoFactory(numberGenerator);
InputView inputView = new InputView(new Scanner(System.in));
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Scanner는 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.

저는 이렇게 의존하는 객체는 최대한 외부로부터 받아오는게 저 좋을것 같아 위처럼 구현했어요. 아무래도 이 방식대로 구현하면 테스팅하기도 좋고, 의존하는 객체가 의존되는 객체의 생성 과정을 몰라도 되어 개발할 때 더 수월하다는 점들 때문에, 왠만하면 의존하는 객체는 외부로부터 받아오고 있도록 개발하는 방향이 더 바람직해 보입니다...!


public class Main {
public static void main(String[] args) {
LottoBatch lottoBatch = new LottoBatch(new ArrayList<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.

#156 (comment)

위 리뷰의 답변과 비슷한 맥락으로 구현하긴 했는데, 아무래도 일급 컬랙션은 비어있는 상태로 생성되는게 더 자연스러운것 같습니다! 수정했습니다!

System.out.println(ScriptConstants.OUTPUT_STAT_HEADER_SCRIPT);

for (LottoResult currentResult : WINNING_RESULT) {
int resultCount = (int) matchCountPerLotto.stream().filter(result -> currentResult == result).count();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

계산을 하는 로직은 outputview에서 이루어지면 안될 것 같아요!

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.

이 부분이 계산하는 것은 인자로 받은 LottoResult 리스트에서 출력해야할 LottoResult를 카운팅하는 것이기에 outputview의 역할에 크게 벗어나지 않아보입니다.

다만 이렇게 될 경우 현재는 outputview에 출력해야할 값들을 List<LottoResult> WINNING_RESULT = List.of(LottoResult.THREE, LottoResult.FOUR, LottoResult.FIVE, LottoResult.SIX); 이렇게 하드코딩 해 넣어주고있는데요, 도메인의 책임을 view가 조금 알게된다는 단점이 있을 것 같아요. 이 부분은 해당 로직을 다른 부분으로 옮김으로써 해결할 수 잇을 것 같습니다~

하지만 위 리뷰를 반영하며 코드를 수정하면서 위 로직이 자연스럽게 컨트롤러로 빠지네요...

Comment on lines +20 to +21
public final int matchCount;
public final int reward;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

필드의 접근제어자가 public인건 위험한 것 같아요!

Copy link
Copy Markdown
Author

@Cappucciyes Cappucciyes Apr 2, 2026

Choose a reason for hiding this comment

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

원시값에 final이라 별 생각 없이 public으로 접근제어자로 두었습니다. 제가 알기론 이런 형태에서는 외부로 노출되어도 상태가 오염될 여지도 없는 것으로 알고 있어요. 혹시 이 상화에서 public으로 접근제어자를 사용하면 또다른 위혐이 있을까요...?

질문 별개로, 일단 private 및 getter를 사용하는 방식으로 수정하였어요!

System.out.println();
}

public void printStats(List<LottoResult> matchCountPerLotto) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

위의 printPurchaseResult메서드는 객체를 dto에 담아 출력을하도록 작성이되었는데, 현재 코드에서는 enum을 그대로 넘기고있네요!
위와 통일성을 주는것이 좋을 것 같아요(둘다 dto vs 둘다 도메인)

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.

참고하고 수정하였습니다! dto 역할 특성상 dto로 통일하는 방향이 더 일관성 있어보여 모두 dto를 받아오는 형식으로 수정했어요.

@Cappucciyes
Copy link
Copy Markdown
Author

꼼꼼한 리뷰에 감사드립니다. 최대한 리뷰를 반영하며 코드를 수정해 봤어요.

해당 질문에 답변을 드리기전에 태규님은 어떤 경우에 Mock객체를 만드셨나요? Mock객체란 왜 사용하는 걸까요?

저는 Mock 객체를 2가지 목적을 가지고 구현하였습니다.

  • 통합 테스트에서 올바른 함수가 올바를 횟수만큼 확인하기 위해
  • private 함수여도 반환값을 확인할 필요가 있어보이는 함수의 메소드를 확인하기 위해.

일단 현제 제가 미션을 진행하면서 Mock객체들을 하나의 페키지로 묶어 관리했는데, 해윤님은 테스트용 Mock 객체를 어떻게 관리하시나요? 아니면 일반적으로 Mock 객체들을 관리할 때 쓰는 방법이 있을까요...?

또한 코드를 수정을 하며 메소드 오버로딩에 대해 질문이 생겼습니다. 입력받은 돈 만큼 Lotto를 생성하는 책임을 LottoFactory에게 옯기면서, 반환된 List를 한번에 LottoBatch에 어떻게 삽입할까 고민한 결과, 그냥 add 메소드를 오버로딩하는 방식으로 구현했습니다. 제가 보기에는 메소드명 의미상 문제 없어보여 좋았지만, 오버로딩은 협업하는 관점에서 오버로딩 된 함수는 어떻게 보면 햇갈림을 유도할 수 있는 기법이라 생각이 드네요. 해윤님은 메소드 오버로딩에 대해 어떻게 생각하시나요?

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