[로또] 김민정 미션 제출합니다. #4
Conversation
| @@ -0,0 +1,7 @@ | |||
| public class GameCountDecider { | |||
|
|
|||
| static final Integer LOTTO_PRICE = 1000; | |||
There was a problem hiding this comment.
LOTTO_PRICE 상수를 같은 패키지에서 사용하지 않을거라면 private 접근제어자로 정의해주는 게 좋을 것 같은데 어떻게 생각하시나요?
| public Integer validatePurchasePrice(String input) { | ||
| try { | ||
| return Integer.parseInt(input); | ||
| } catch (NumberFormatException e) { | ||
| System.out.println("숫자를 입력해주세요"); | ||
| return null; | ||
| } | ||
| } |
There was a problem hiding this comment.
요 메서드는 네이밍만 보면 구매 금액에 대해 validate를 해준다는 의미인 것 같은데 실제로는 null을 반환하고 있고 해당 메서드를 호출하는 외부에서 null에 대해서 do while 문으로 처리를 해주고 있어요~
validate를 하고자 한다면 유효성 검사 후 Exception을 발생시켜 외부에서는 해당 Exception에 대해 처리하도록 하는 건 어떨까요?
추가로 validate 역할을 수행하는 Validator 객체를 만들어 해당 객체에서 처리하도록 하는 것도 고려해보셨으면 해요😄
| int minimumLottoValue = 1; | ||
| int maximumLottoValue = 46; | ||
| int lottoNumberCount = 6; |
There was a problem hiding this comment.
해당 값들은 Lotto 라는 도메인에서 종속된 값이기 때문에 Lotto 객체에서 상수로 가지고 있어도 무방할 값으로 보여요!
| @Override | ||
| public String toString() { | ||
| return gameNumbers.toString(); | ||
| } |
There was a problem hiding this comment.
혹시 해당 toString() 메서드는 로깅 용도로 사용하기 위해 오버라이딩 하셨나요? 오버라이딩 하신 이유가 궁금해요!
| Integer price = acceptPriceForLottoGame(); | ||
| Integer gameCount = purchaseGamesByPrice(price); |
There was a problem hiding this comment.
Integer를 사용하고 있어 최종적으로 printPurchasedGameCount() 메서드에서 결과값을 보여줄 때 null 값이 출력될 수도 있을 것 같아요!
null 에 대한 상태가 필요해서 Integer 타입으로 하신 건지 궁금해요!
| System.out.println("3개 일치 (" + profitCalculator.FIFTH_PRIZE + "원)- " + winners.getFifthWinner()+"개"); | ||
| System.out.println("4개 일치 (" + profitCalculator.FOURTH_PRIZE + "원)- " + winners.getFourthWinner()+"개"); | ||
| System.out.println("5개 일치 (" + profitCalculator.THIRD_PRIZE + "원)- " + winners.getThirdWinner()+"개"); | ||
| System.out.println("5개 일치, 보너스 볼 일치 (" + profitCalculator.SECOND_PRIZE + "원)- " + winners.getSecondWinner()+"개"); | ||
| System.out.println("6개 일치 (" + profitCalculator.FIRST_PRIZE + "원)- " + winners.getFirstWinner()+"개"); |
There was a problem hiding this comment.
위 ProfitCalculator 객체에 단 리뷰에서처럼 등수에 대한 내용을 Enum으로 관리한다면 각 등수 별 일치 메세지도 담아서 처리해줄 수 있을 것 같아요~😄
| @@ -0,0 +1,89 @@ | |||
| import java.util.List; | |||
|
|
|||
| public class WinnerSelector { | |||
There was a problem hiding this comment.
해당 객체에서도 등수를 계산하기 위해 각각의 로또 번호의 매칭된 카운트를 직접 계산해주고 있는데, 이 부분도 Enum에서 매칭 카운트 값을 등수 값과 함께 관리하고 있다면 더 용이하게 구현해볼 수 있을 것 같아요!
| resultView = Mockito.mock(ResultView.class); | ||
| inputView = Mockito.mock(InputView.class); |
There was a problem hiding this comment.
Mockito를 사용해서 테스트코드를 작성하신 부분 잘 보고 갑니다!👍
| private final Integer fourthWinner; | ||
| private final Integer fifthWinner; | ||
|
|
||
| public Winners(Integer firstWinner, Integer secondWinner, Integer thirdWinner, Integer fourthWinner, Integer fifthWinner) { |
There was a problem hiding this comment.
위에서도 비슷한 리뷰를 달았습니다만, 민정님은 int와 Integer는 어떤 기준으로 선택하시는지 궁금해요!
| int firstWinner = 0; | ||
| int secondWinner = 0; | ||
| int thirdWinner = 0; | ||
| int fourthWinner = 0; | ||
| int fifthWinner = 0; |
There was a problem hiding this comment.
최대한 불변 변수를 사용해서 값을 구해보는 걸 추천드릴게요!
이는 내 코드가 그렇게 이상한가요? 도서에 4장. 불변 활용하기 장에서 자세한 내용을 다루고 있습니다!
가장 큰 이유는 가변 변수를 사용하게 되면 계속 변경되는 변수의 값을 따라가기 어려워지고, 변수의 값을 혼동할 수 있다는 점이 제일 와 닿았어요!
기능 목록 정리
입력
출력
로또 생성
당첨 조회
당첨 통계