로또 4단계 - 로또(수동) PR반영(3단계 PR포함) #4150
Conversation
| - [x] 수동/자동 구매 정보 화면 노출 | ||
| - [x] 로또 타입에 자동/수동 구분 짓는 상태에 대한 고민 | ||
| - [x] LottoMachine클래스의 buy 메서드에서 추상화 수준 동일하게 리팩토링 | ||
| - [x] LottoNumberMatcher 대신에 로또 번호와 당첨결과를 가지는 Lottos클래스 구조에 대한 고민/검토 | ||
| - [x] 로또 정답 당첨 테스트에서 특정 케이스(예: 3개+보너스 번호 일치) 케이스에 대한 검토, 그외에 보너스 번호 관련 테스트 코드 전반적인 점검 | ||
| - [x] 현재 로또 생성기는 자동 생성 기능만 제공한다. 사용자가 수동으로 추첨 번호를 입력할 수 있도록 해야 한다. | ||
| - [x] 입력한 금액, 자동 생성 숫자, 수동 생성 번호를 입력하도록 해야 한다. |
| private int manualLottoCount; | ||
| private int autoLottoCount; |
There was a problem hiding this comment.
count 를 상태로 갖고있게 해주셨군요!
수동으로 3장, 자동으로 11개를 구매했습니다. 이 문구를 출력하기 위해 상태로 갖고있는것 같아요
이런 구조라면 로또의 타입이 추가될때마다 카운트 변수값이 계속 추가되는 문제가 발생합니다 🤔
출력하기 위해 값을 꺼낼때 값을 제공해주는 방법도 있을것 같아요!
public int autoLottoCount() {
return (int) lottoNumbers.stream()
.filter(LottoNumbers::isAuto)
.count();
}그리고 Lotto 엔 LottoType 과 같은 enum 을 갖고 있게 하면 될것 같아요 :)
이렇게하면 카운트변수가 없어도 되고 컬렉션의 요소가 변경되어도 카운트를 매번 갱신하거나 관리해줄 필요가 없는 장점도 있습니다!
지금 구현해주신 방법이 틀렸다는게 아니고 이러한 방법도 있다고 생각하시면 될것 같아요 😄
There was a problem hiding this comment.
맞습니다. 동의합니다.
현재까지 현실에서 사용되는 로또의 비즈니스에서 더 이상 타입 추가가 없다는 가정하에 작성된 코드이고,
타입이 추가된다고 하면 리팩토링을 통해 타입을 추가하는 방향으로 갔을 겁니다 :)
저도 타입 추가를 할까 말까 고민한 부분이라 해당 부분에 대해 얘기나누니 좋군요~
| private static Map<LottoRank, Integer> initializeLottoWinningBoardMap() { | ||
| Map<LottoRank, Integer> rankMap = new EnumMap<>(LottoRank.class); | ||
| for(LottoRank lottoRank: LottoRank.values()) { | ||
| rankMap.put(lottoRank, 0); | ||
| } | ||
| return rankMap; | ||
| } |
There was a problem hiding this comment.
현재 LottoWinningRecord의 생성 역할을 Lottos가 담당하고 있네요 🤔
각 객체의 책임과 역할을 더 명확히 분리하면 좋을 것 같습니다!
public LottoWinningRecord match(WinningLottoNumbers winningLottoNumbers) {
LottoWinningRecord lottoWinningRecord = new LottoWinningRecord();
lottoNumbers.forEach(lottoNumber -> {
LottoRank lottoRank = winningLottoNumbers.lottoRank(lottoNumber);
lottoWinningRecord.add(lottoRank);
});
return lottoWinningRecord;
}이렇게 변경하면 LottoWinningRecord는 생성자에서 초기화를 담당하고, add 메서드를 통해 각 등수별 카운트를 증가시키는 책임을 명확히 할 수 있습니다. (add -> record 가 더 적절할수도 있겠네요 🤔)
이는 단일 책임 원칙(SRP)에도 위반되지 않고 코드의 가독성과 유지보수성도 향상될 것 같아요!
| public void 로또번호_정답당첨_숫자3개일치_보너스번호일치() { | ||
| LottoNumbers numbers = LottoNumbers.fromIntegers(Arrays.asList(1, 2, 3, 4, 5, 6)); | ||
| LottoNumber bonusNumber = new LottoNumber(6); | ||
| WinningLottoNumbers winningNumbers = new WinningLottoNumbers( | ||
| LottoNumbers.fromIntegers(Arrays.asList(1, 2, 3, 10, 11, 12)), | ||
| bonusNumber); | ||
|
|
||
| assertThat(winningNumbers.lottoRank(numbers)).isEqualTo(MATCH_3); | ||
| } | ||
|
|
||
| @Test | ||
| public void 로또번호_정답당첨_숫자4개일치_보너스불일치() { | ||
| LottoNumbers numbers = LottoNumbers.fromIntegers(Arrays.asList(1, 2, 3, 4, 5, 6)); | ||
| LottoNumber bonusNumber = new LottoNumber(13); | ||
| WinningLottoNumbers winningNumbers = new WinningLottoNumbers( | ||
| LottoNumbers.fromIntegers(Arrays.asList(1, 2, 3, 4, 11, 12)), | ||
| bonusNumber); | ||
|
|
||
| assertThat(winningNumbers.lottoRank(numbers)).isEqualTo(MATCH_4); | ||
| } | ||
|
|
||
| @Test | ||
| public void 로또번호_정답당첨_숫자4개일치_보너스번호일치() { | ||
| LottoNumbers numbers = LottoNumbers.fromIntegers(Arrays.asList(1, 2, 3, 4, 5, 6)); | ||
| LottoNumber bonusNumber = new LottoNumber(6); | ||
| WinningLottoNumbers winningNumbers = new WinningLottoNumbers( | ||
| LottoNumbers.fromIntegers(Arrays.asList(1, 2, 3, 4, 11, 12)), | ||
| bonusNumber); | ||
|
|
||
| assertThat(winningNumbers.lottoRank(numbers)).isEqualTo(MATCH_4); | ||
| } |
There was a problem hiding this comment.
테스트코드 잘 작성해주셨네요! 👍
평소 감명깊게 읽은 아티클 하나 추천해드릴게요!
시간나실때 읽어보시는것 추천드립니당
| @Test | ||
| public void 로또번호_동등성_테스트() { | ||
| LottoNumbers numbers1 = LottoNumbers.fromIntegers(Arrays.asList(1, 2, 3, 4, 5, 6)); | ||
| LottoNumbers numbers2 = LottoNumbers.fromIntegers(Arrays.asList(1, 2, 3, 4, 5, 6)); | ||
|
|
||
| assertThat(numbers1.equals(numbers2)).isTrue(); | ||
| } |
src/main/java/lotto/LottoGame.java
Outdated
| if (manualPurchaseLottoCount * PRICE_OF_LOTTO > amount) { | ||
| throw new IllegalArgumentException("수동 구매 가격이 구입금액보다 클 수 없습니다."); | ||
| } |
There was a problem hiding this comment.
금액을 관리하는 LottoMachine 의 책임이 더 어울릴것 같아요!
src/main/java/lotto/LottoGame.java
Outdated
| List<String> manualLottoNumberInputs = new ArrayList<>(); | ||
| if (manualPurchaseLottoCount > 0) { | ||
| manualLottoNumberInputs = InputView.inputManualLottoNumbers(manualPurchaseLottoCount); | ||
| } |
There was a problem hiding this comment.
InputView에서 빈 리스트를 반환하도록 수정하면 코드가 더 간결해질 것 같아요 🤔
| List<String> manualLottoNumberInputs = new ArrayList<>(); | |
| if (manualPurchaseLottoCount > 0) { | |
| manualLottoNumberInputs = InputView.inputManualLottoNumbers(manualPurchaseLottoCount); | |
| } | |
| List<String> manualLottoNumberInputs = InputView.inputManualLottoNumbers(manualPurchaseLottoCount); |
이렇게 하면 inputManualLottoNumbers 메서드 내부에서 manualPurchaseLottoCount가 0 이하일 경우 빈 리스트를 반환하도록 구현하면 될 것 같아요!
입력 처리 로직이 InputView에 완전히 캡슐화되어 책임 분리가 더 명확해질 것 같습니다! 👍
리뷰 중점 부분
기능 요구사항
프로그래밍 요구사항