[로또] 박주희 미션 제출합니다. #1366
[로또] 박주희 미션 제출합니다. #1366
Conversation
There was a problem hiding this comment.
LottoTicket과 Lotto 클래스가 인스턴스 변수로 정수의 리스트를 갖고 있는데
조금은 역할이 겹쳐 보이기도 합니다.
| private static void validateMaximumPurchaseAmount(int purchaseAmount) { | ||
| if (purchaseAmount > MAX_PURCHASE_AMOUNT) { | ||
| throw new LottoException(LottoErrorMessage.EXCEEDS_MAX_PURCHASE_AMOUNT); | ||
| } | ||
| } |
| NO_MATCH(0, 0, false), | ||
| THREE_MATCH(3, 5000, false), | ||
| FOUR_MATCH(4, 50000, false), | ||
| FIVE_MATCH(5, 1500000, false), | ||
| FIVE_MATCH_WITH_BONUS(5, 30000000, true), | ||
| SIX_MATCH(6, 2000000000, false); |
There was a problem hiding this comment.
숫자 구분을 위해 3자리마다 _를 넣어주면 좋을 것 같습니다.
1_500_000처럼 나타낼 수 있습니다.
| InputView inputView = new InputView(); | ||
| OutputView outputView = new OutputView(); | ||
| LottoController lottoController = new LottoController(inputView, outputView); |
There was a problem hiding this comment.
애플리케이션 내에서 보면 주희 님이 생성하신 인스턴스가 다시 할당되지 않기 때문에 inputView, outputVie, lottoController에 final을 명시하는 것은 어떨지 궁금해요!
|
|
||
| public LottoController(InputView inputView, OutputView outputView) { | ||
| this.lottoService = new LottoService(inputView, outputView); |
There was a problem hiding this comment.
mvc 패턴에서 모델과 뷰의 관계에 대해 생각해 보시면 좋을 것 같아요
There was a problem hiding this comment.
패키지명은 최대한 단수형을 지정하는 것도 컨벤션의 일부인 것을 아셨나요?.? 저도 방금 알았습니다...
| import java.util.Map; | ||
|
|
||
| public class LottoMachine { | ||
| private final Lotto winningLottoNumbers; |
There was a problem hiding this comment.
이넘 클래스 내에서도 메소드를 구현할 수 있다는 걸 얼마 전에 알게 되었는데 주희 님께서는 이부분을 잘 활용하신 것 같아서 인상 깊습니다 😋
| public static int validateBonusNumber(String bonusNumberInput) { | ||
| validateEmptyOrBlank(bonusNumberInput); | ||
| int bonusNumber = parseBonusNumber(bonusNumberInput); | ||
| bonusNumberRange(bonusNumber); | ||
|
|
||
| return bonusNumber; | ||
| } |
There was a problem hiding this comment.
bonusNumberInput에 대한 null과 공백값에 대한 검증을 먼저 마친 뒤에 int로 반환해 주는 걸로 보이는데, 이럴 경우 숫자가 아닌 값이 들어왔을 때 NumberFormatException이 발생할 수도 있을 것 같습니다 이 부분은 따로 검증하는 곳이 있는 걸까요?
| private static void validateEmptyOrBlank(String input) { | ||
| if (input.strip().isBlank()) { | ||
| throw new LottoException(LottoErrorMessage.EMPTY_OR_BLANK_INPUT); | ||
| } | ||
| } |
| } | ||
|
|
||
| private static List<Integer> parseWinningNumbers(String winningNumbersInput) { | ||
| return List.of(winningNumbersInput.split(",")) |
There was a problem hiding this comment.
만약 구분자가 쉼표가 아닌 다른 것으로 바뀐다면 해당 메소드 내부까지 수정해야 해서 ","보다 상수로 가지고 계셔도 괜찮을 것 같아요
| private static final int MIN_NUMBER = 1; | ||
| private static final int MAX_NUMBER = 45; |
There was a problem hiding this comment.
검증 부분에서 의미와 값이 중복되어 나타나는 상수는 따로 enum을 통해 관리하는 것도 괜찮지 않을까 합니다!
There was a problem hiding this comment.
LottoValidation에 있는 메소드들과 중복되는 메소드가 많은 것 같아서 그런 메소드들은 두 클래스가 동일하게 사용할 수 있도록 가져가는 것도 괜찮을 것 같아요! 저 같은 경우에는 LottoValidation이라는 클래스 내부에 중복되는 메소드를 protected로 제어해 놓고, 해당 클래스를 LottoNumberValidator 같은 클래스가 상속하게 해서 public 메소드 안에서 사용할 수 있도록 했는데 올바른 방법일지 모르겠지만 괜찮았다고 느껴서 이야기해 보았습니다 😇
No description provided.