2단계: 로또(자동) #3798
2단계: 로또(자동) #3798
Conversation
| @@ -0,0 +1,19 @@ | |||
| package step2.enums; | |||
|
|
|||
| public enum ExceptionMessage { | |||
There was a problem hiding this comment.
enum으로 관리하는 것도 나쁘진 않으나 이 메시지가 정말 필요한 클래스에 넣으면 응집력을 높이는데 도움이 됩니다.
어느 쪽이 코드를 관리하기 쉬운지 고민해보시면 좋을 것 같아 남겨봅니다.
| @@ -0,0 +1,21 @@ | |||
| package step2.enums; | |||
|
|
|||
| public enum Message { | |||
There was a problem hiding this comment.
클래스명이 너무 일반적이네요.
특징을 유추할 수 있도록 변경해주세요.
| if (lotto.contains(Integer.parseInt(s))) { | ||
| matchCount++; | ||
| } |
There was a problem hiding this comment.
프로그래밍 요구사항
- indent(인덴트, 들여쓰기) depth를 2를 넘지 않도록 구현한다. 1까지만 허용한다.
- 예를 들어 while문 안에 if문이 있으면 들여쓰기는 2이다.
힌트: indent(인덴트, 들여쓰기) depth를 줄이는 좋은 방법은 함수(또는 메소드)를 분리하면 된다.
- 예를 들어 while문 안에 if문이 있으면 들여쓰기는 2이다.
2 depth가 들어가 있어요.
| private final List<List<Integer>> lottos = new ArrayList<>(); | ||
| private final PrizeStatics prizeStatics = new PrizeStatics(); | ||
|
|
||
| private final int money; |
There was a problem hiding this comment.
money가 진짜 필요한 필드일까요?
수익률을 가져오는 데만 사용하고 있거든요.
lottos로도 충분히 유추할 수 있어보이기도 합니다.
| public PrizeStatics getPrizeStatics() { | ||
| return prizeStatics; | ||
| } |
There was a problem hiding this comment.
PrizeStatics 도 필드로 지정되어 있는데 굳이 필드로 지정할 필요는 있는지도 고민해보시면 좋겠습니다.
필드로 지정되는 이유는 이 객체 안에서 사용하기 때문만은 아니에요.
우리는 현실을 코드로 작성하는 업무를 하고 있습니다.
대상이 되는 그 현실의 특징을 반영하는게 객체지향이구요.
그럼 생각을 다시 해서, 우리가 로또를 구매할 때 합격을 기대하지만 그 로또 자체에서 합격을 알 수가 있나요?
추첨 방송이나 인터넷을 확인해야만 당첨 여부를 확인할 수 있어요.
로또 입장에서 주체적으로 당첨 여부를 확인하는건 앞뒤가 맞지 않을 수 있다는 것이죠.
그리고 로직 상에서도 사람의 실수로 인해 버그가 발생할 가능성이 있습니다.
addLotto를 통해 Lotto 의 개수는 계속 변경됩니다.
하지만 checkPrizeNum을 호출하지 않으면 PrizeStatics 에 반영되지 않고 있어요.
만약 누군가의 실수로 저 메소드를 호출하지 않고 getPrizeStatics를 바로 불러낸다면 어떻게 될까요?
차라리 getPrizeStatics 에서 매번 현재의 로또 상태를 만들어서 반환하는게 더 나을 수 있습니다.
이런 점들을 잘 고려하여 필드로 넣어두는게 타당한지 한번 더 고민해주시기를 부탁드립니다.
| //번호가 1~45 범위에 포함되는지 체크한다. | ||
| private void checkNum(int num) { | ||
| if (num < MINIMUM_LOTTO_NUM || num > MAXIMUM_LOTTO_NUM) { | ||
| throw new IllegalArgumentException(ExceptionMessage.RANGE_LOTTO_NUM.message()); | ||
| } | ||
| } |
There was a problem hiding this comment.
이 책임은 Lotto 보다는 LottoMachine 이 더 어울리지 않을까요?
로또를 사면 이미 번호가 찍혀있는 상태로 우리의 손에 들어옵니다.
심지어 로또를 수동으로 산다면 우리가 찍을 수 있는 번호도 이미 정해져있는 채로 구매를 결정합니다.
그런걸 생각해보면 1 ~ 45를 검사하는건 로또머신의 역할에 더 가까워보입니다.
| private final static int MINIMUM_LOTTO_NUM = 1; | ||
| private final static int MAXIMUM_LOTTO_NUM = 45; | ||
|
|
||
| private final List<List<Integer>> lottos = new ArrayList<>(); |
There was a problem hiding this comment.
중복을 제거하려면 Set<Integer> 가 더 낫지 않을까요?
| lotto.addLotto(Arrays.asList(1, 2, 3, 4, 5, 6)); | ||
| lotto.addLotto(Arrays.asList(7, 8, 9, 10, 11, 12)); |
There was a problem hiding this comment.
과제는 java 11 기반이니 콜렉션 생성자 사용하시는게 더 편할거에요.
| lotto.addLotto(Arrays.asList(1, 2, 3, 4, 5, 6)); | |
| lotto.addLotto(Arrays.asList(7, 8, 9, 10, 11, 12)); | |
| lotto.addLotto(List.of(1, 2, 3, 4, 5, 6)); | |
| lotto.addLotto(List.of(7, 8, 9, 10, 11, 12)); |
안녕하세요 리뷰어님,
리뷰 부탁드립니다.
아래와 같이 역할을 담당시켰습니다!
LottoMachine.java -> 로또를 발급해주는 기계
Lotto.java -> 로또를 구매한 사람
PrizeStatics -> 로또 당첨 통계를 담당
좋은 주말 보내십시오!