[로또] 정현섭 미션 제출합니다. #1352
[로또] 정현섭 미션 제출합니다. #1352
Conversation
- 구매 금액에 따른 로또 구매 개수 검증 - 로또 하나당 크기가 6인지 검증 - 로또에 들어가는 숫자가 0에서 45 사이인지 검증
- PRICE - PURCHASE_LIMIT - NUMBERS_COUNT - MIN_NUMBER - MAX_NUMBER
- Purchase와 Lotto에서 쓰였던 문자열을 정수로 변환하는 로직을 LottoService에서 실행 - Purchase와 Lotto는 데이터 검증만 담당 - 그에 따른 테스트 로직 수정
- 매개 변수의 자료형을 List에서 int로 변경 - 기존에 List를 받아서 한번에 모두 검증 하던 것을 하나씩 검증
- assignWinningNumbers의 접근 제어자를 private로 변경
| public class Application { | ||
| public static void main(String[] args) { | ||
| // TODO: 프로그램 구현 | ||
| LottoController lottoController = new LottoController(new LottoServiceImpl(), new ConsoleLottoView()); | ||
| lottoController.run(); | ||
| } |
There was a problem hiding this comment.
main 메서드에서 의존성 주입을 통해 객체를 생성하고 연결하셨군요! 👍
| public static final int MAX_NUMBER = 45; | ||
|
|
||
| private LottoConstants() { | ||
| } |
There was a problem hiding this comment.
불필요한 인스턴스화 방지를 위해 생성자를 private으로 정의하셨군요!
| throw new IllegalArgumentException("[ERROR] 100,000원까지만 구매가 가능합니다."); | ||
| } | ||
| if (amount < LottoConstants.PRICE) { | ||
| throw new IllegalArgumentException("[ERROR] 1,000원 이상의 금액을 입력하셔야 합니다."); |
There was a problem hiding this comment.
의미있는 값들을 상수화하셨던 것처럼, 예외 메시지도 상수화하시면 더 정확(오타 방지)하고 일관적이고 유지보수 높게 메시지를 드러낼 수 있을 것 같습니다!
| } | ||
| } | ||
|
|
||
| public static void validateNumberExists(List<Integer> numbers, Integer number) { |
There was a problem hiding this comment.
List로 로또 번호들을 단순히 int값으로 취급하고 있으신데, LottoNumber와 같은 객체를 활용하여 List로 검증할 수 있다면 더 객체지향적인 코드가 될 것 같습니다.
| private LottoValidateUtil() { | ||
| } | ||
|
|
||
| public static void validatePurchaseAmount(int amount) { |
There was a problem hiding this comment.
해당 메서드에서 금액 제한을 100,000원, 1,000원 단위로 나누어 검사하고 있는데, 각 에러 상황별로 구체적인 메서드로 분리하면 더 구체적이고 읽기 쉬운 코드가 될 것 같습니다.
| String amount = lottoView.readPurchaseAmount(); | ||
| lottoService.buyLotto(amount); | ||
| } catch (IllegalArgumentException e) { | ||
| System.out.println(e.getMessage()); |
There was a problem hiding this comment.
예외 발생 시 System.out.println(e.getMessage());를 통해 예외 메시지를 출력하고 있는데, lottoView의 출력 메서드를 통해 메시지를 출력하도록 하면 Controller는 뷰에 대한 구체적인 구현을 몰라도 되어 더 일관성 있는 코드가될 것 같습니다 😄
ex) lottoView.displayError(e.getMessage());
| private final LottoService lottoService; | ||
| private final LottoView lottoView; | ||
|
|
||
| public LottoController(LottoService lottoService, ConsoleLottoView lottoView) { |
There was a problem hiding this comment.
이렇게 LottoController가 ConsoleLottoView에 의존하게 되면, LottoView의 다른 구현체를 사용하기 어려워집니다..! LottoView 인터페이스를 정의한 목적은 유연성과 확장성을 높이기 위해서인데, 현재 코드는 특정 구현체에 고정되어 있어 인터페이스의 장점을 제대로 활용하지 못하고 있습니다. 😢
LottoController의 생성자에서 ConsoleLottoView 대신 LottoView 타입을 받아들이도록 수정하면 더 좋을 것 같습니다!
| int parsedAmount; | ||
|
|
||
| if (amount == null || amount.isBlank()) { | ||
| amount = ""; |
There was a problem hiding this comment.
if (amount == null || amount.isBlank()) { amount = ""; } 이 부분이 사실상 불필요해보입니다. amount가 null이거나 빈 문자열인 경우 예외를 바로 던지는 편이 더 명확하고 안전할 것 같습니다!
| return amount; | ||
| } | ||
|
|
||
| private int convertStringToInt(String amount) { |
There was a problem hiding this comment.
입력된 문자열을 파싱하고 유효성을 검사하는 로직이다 보니, 메서드 이름을 'parseAndValidateAmount'와 같이 더 명확히 해주면 좋을 것 같습니다~
| if (amount == null || amount.isBlank()) { | ||
| amount = ""; | ||
| } | ||
| String cleanAmount = amount.replace(" ", ""); |
There was a problem hiding this comment.
amount의 문자열을 클린업(clean-up)하는 과정이 약간 복잡해 보입니다. 기본적인 trim()을 사용하여 양쪽 공백만 제거하거나, 전체 공백 제거가 필요하다면 replaceAll("\s+", "")로 단순화할 수 있습니다..!
No description provided.