[빙봉] 로또 2단계 - 수동 구매 미션 제출합니다. #196
Conversation
- 로또 티켓 갯수를 구해주는 calculateLottoTicket() 메서드 추가 - refactor : WinningLottoTicket 클래스에서 정규표현식 제거
- WinningLottoTicket 클래스에 private 접근제한자 추가
- LottoNubmers 클래스명을 LottoNumbersGenerator로 변경 - getTotal 메서드명 수정 - getCorrectCount 메서드에서 인덱스로 비교하는 것을 contains로 수정
- String.Utils의 isBlank 메서드를 spark 패키지에서 apache 패키지로 변경
…, WinningLottoTicket의 생성자에서 bonusBall을 받도록 수정
…toManager 클래스 추가, 테스트코드 수정 - LottoResult 클래스 명을 LottoCount로 변경 - LottoResults 클래스에서 add() 메서드 삭제 - LottoTicket 클래스에 있던 getCorrectCount() 메서드를 WinningLottoTicket 클래스로 옮김 - LottoTickets 클래스에 있던 match() 메서드 삭제 - LottoResultsTest 클래스 삭제 - LottoTicketsTest 클래스 삭제
- LottoCount에 대한 메서드 삭제 - LottoResults 클래스의 putLottoResults 메서드 수정 - OutputView에 printWinningStatistics 메서드에서 Map을 반복할 때 entry 방법이 아닌 foreach로 변경
- List를 메서드 안에서 만들고 반환하도록 로직 수정
…ator 에서 같은 인스턴스로 가져오도록 getter 메서드 추가, OutputView 에서 Map이 아닌 RankType으로 출력 수정
…변경 및 메서드 추가, LottoResults 테스트 코드 작성
…머지 클래스에서 getLottoNumber 메서드 수정, getLottoNumber 메서드 테스트 코드 추가
…스 변수명 수정 및 삭제, Money 클래스의 메서드 명 수정,
…추가, bonusBall 의 네이밍을 bonusNumber로 수정,
…TicketGenerator 클래스 추가 및 테스트코드 작성, refactor : LottoNumbersGenerator 클래스명 수정, WinningLottoTicket 의 생성자 로직 리펙토링 - LottoNumbersGenerator 클래스명을 LottoTicketsGenerator 로 변경 - LottoTicket 안에서 LottoNumber 를 찾을 수 있는 containsLottoNumber() 메서드 추가 - LottoTicketGenerator 클래스 추가 및 문자열로 받았을 때 로또 티켓을 생성할 수 있는 createLottoTicket() 메서드 추가 - WinningLottoTicket 클래스의 생성자 로직 리펙토링 -
…경 및 테스트 코드 추가, Money 클래스에 있는 countLottoTicket() 메서드 삭제, InputView 클래스에 수동 로또 입력 로직 작성, OutputView 클래스에 수동 로또 출력 로직 작성, MoneyTest 클래스에서 구매한 로또 총 갯수를 테스트하는 로직 삭제 - ManualBuyLottoTicketCount 클래스를 ManualLottoTicketQuantity 로 변경
jihan805
left a comment
jihan805
left a comment
There was a problem hiding this comment.
안녕하세요 전 단계 피드백 반영 잘하셨어요 💯
몇가지 피드백 남겼으니 확인 해주세요 :)
| @Override | ||
| public int hashCode() { | ||
| return Objects.hash(lottoNumber); | ||
| public static LottoNumber getLottoNumber(int number) { |
There was a problem hiding this comment.
get~하는대신, LottoNumber 생성을 위한 정적 팩터리 메서드를 만들어 보면 어떨까요?
|
|
||
| public static LottoTickets generateLottoTicketsByMoney(Money money) { | ||
| return new LottoTickets(LottoNumbersGenerator.generateLottoTickets(money.calculateLottoTicket())); | ||
| public static void LottoTicketsResult(Money money, LottoTickets lottoTickets) { |
There was a problem hiding this comment.
메서드 네이밍 lottoTicketsResult~, 아래 모두 동일!
| OutputView.printWinningStatistics(LottoManager.match(lottoTickets, inputWinningLottoTicket()), money); | ||
| } | ||
|
|
||
| public static void LottoTicketsCount(Money money, ManualLottoTicketQuantity manualLottoTicketQuantity) { |
There was a problem hiding this comment.
메서드로 분리하고 싶다면, buyManualLotto등의 네이밍이 더 적합하지 않을까요?
| OutputView.printBuyTicketCount(money, manualLottoTicketQuantity); | ||
| } | ||
|
|
||
| public static void LottoTicketsStatus(LottoTickets lottoTickets) { |
There was a problem hiding this comment.
이와같은 경우, 메서드로 분리하지 않아도 괜찮습니다 :)
이전 피드백의 경우
OutputView.printWinningStatistics(LottoManager.match(lottoTickets,inputWinningLottoTicket()).getLottoResults(), money.getMoney())
이 한줄에 축약된 행위들이 너무 많아, 조금 분리하여 가독성을 높이면 어떨까?에 대한 피드백이었습니다.
LottoTicketsResult() 메서드 역시 동일하구요 !!
src/main/java/view/InputView.java
Outdated
| public static void inputManualLottoTicket(ManualLottoTicketQuantity manualLottoTicketQuantity) { | ||
| System.out.println("수동으로 구매할 번호를 입력해주세요."); | ||
| for (int i = 0; i < manualLottoTicketQuantity.getManualLottoTicketQuantity(); i++) { | ||
| LottoTicketsGenerator.addManualLottoTicket(scanner.nextLine()); |
There was a problem hiding this comment.
수동으로 구매한 로또를 InputView에서 생성하는 것이 적절할까요?
InputView 는 입력받는 이상의 역할을 하지 않도록 해요!
|
|
||
| private static final List<LottoNumber> originLottoNumbers; | ||
| private static final List<LottoNumber> shuffleLottoNumbers; | ||
| private static final List<LottoTicket> lottoTickets = new ArrayList<>(); |
There was a problem hiding this comment.
LottoTicketsGenerator가 lottoTickets을 가지고 있을 필요는 없을것 같아요!
아래 힌트를 참고해 수동 & 자동 로또 구매를 위한 설계를 조금 더 고민해보면 어떨까요?
2단계에서 3단계로 요구사항이 변경될 때 로또를 생성하는 부분의 요구사항만 변경되었어요,
로또를 생성하는 부분과 로또 결과를 구하는 부분을 다음과 같은 인터페이스로 분리해 구현해 보면 어떨까요?
(2단계에서 3단계로 요구사항이 변경될 때 변경을 최소화하는 연습)
public interface LottosGenerator {
List<Lotto> generate();
}
…수정, WinningLottoTicket 클래스에 중복되는 validate 메서드 삭제
…Generator 클래스 추가, refactor : InputView 에 비즈니스 로직이 들어가지 않도록 수정, LottoManager 에서 generate 하도록 수정 - AutoCount, ManualCount 클래스를 추가해 LottoTicketCount 의 상태값들을 포장 - AutoLottoGenerator, ManualLottoGenerator 클래스를 추가하고 LottoTicketsGenerator 를 인터페이스로 만들어 구현하도록 함 - InputView와 OutputView 리펙토링
…umber 메서드 명 수정, Money 클래스의 getTotalWinningPrice, getProfit 메서드 명 수정, WinningLottoTicket 클래스의 getCorrectCount 메서드 명 수정
- 상금이 21억을 넘을 경우를 대비해 int 형을 long 형으로 바꿔줌
- OutputView, Money의 역할에 맞게 수정
| @Override | ||
| public int hashCode() { | ||
| return Objects.hash(lottoNumber); | ||
| public static LottoNumber from(String input) { |
| return new LottoResults(lottoTickets, winningLottoTicket); | ||
| } | ||
|
|
||
| public static LottoTickets generateLottoTickets(List<String> manualLottoTickets, AutoCount autoCount) { |
There was a problem hiding this comment.
인터페이스를 이용하여 구현 👍
혹은 두 로또를 합쳐 하나의 로또를 만들어주는 클래스를 분리할수도 있을 것 같아요!
|
|
||
| public class ManualLottoGenerator implements LottoTicketsGenerator { | ||
|
|
||
| List<String> manualLottoTickets; |
| MATCH_FOUR(4, 50_000), | ||
| MATCH_FIVE(5, 1_500_000), | ||
| MATCH_FIVE_WITH_BONUS(5, 30_000_000), | ||
| MATCH_SIX(6, 2_000_000_000); |
There was a problem hiding this comment.
private boolean hasBonus를 가지고,
MATCH_FIVE(5, 1_500_000,false),
MATCH_FIVE_WITH_BONUS(5, 30_000_000, true)
와 같이 구현하는 방법도 있을 것같아요!
| } | ||
|
|
||
| @DisplayName("당첨 복권 번호와 얼마나 맞는지 카운트해주는 메서드 테스트") | ||
| @DisplayName("Should_당첨 카운트(보너스 볼 포함)_When_로또 티켓과 당첨 복권이 주어졌을 때") |
There was a problem hiding this comment.
무엇을 테스트하여 기대하는지를 네이밍하는 것이 중요하지, Should와 When을 꼭 기입하지는 않아도되요! ㅎㅎ
3 participants
많은 피드백 부탁드립니다 :) 감사합니다!