lotto step 2 #4242
lotto step 2 #4242
Conversation
javajigi
left a comment
javajigi
left a comment
There was a problem hiding this comment.
시간이 많이 지났음에도 불구하고 미션 진행 👍
단, 로또 프로그램을 직접 실행하는 main() 메서드와 View 관련 로직은 보이지 않네요.
main()과 View도 추가해 프로그램을 완성해 보면 좋겠습니다.
프로그램을 완성하다보면 현 상태에서 아쉬운 부분이 보일 것 같아요.
| if (numbers.size() != SIZE) { | ||
| throw new IllegalArgumentException("로또 번호는 6개여야 한다."); | ||
| } | ||
| if (numbers.stream().distinct().count() != SIZE) { |
There was a problem hiding this comment.
중복 체크를 고려한다면 처음부터 List가 아닌 Set으로 구현하는 것을 어떨까?
Set으로 관리하다 외부에서 이 값을 접근할 때 순서를 보장해야 한다면 정렬해서 반환하는 것은 어떨까?
| boolean outOfRange = numbers.stream().anyMatch(n -> n < 1 || n > 45); | ||
| if (outOfRange) { | ||
| throw new IllegalArgumentException("로또 번호는 1~45 범위여야 한다."); | ||
| } |
There was a problem hiding this comment.
validate를 한 메서드에서 하기보다 각 성격에 따라 메서드를 분리해 보는 것은 어떨까?
| @@ -0,0 +1,32 @@ | |||
| package lotto; | |||
|
|
|||
| public class Money { | |||
| package lotto; | ||
|
|
||
| public class Money { | ||
| private static final int UNIT_PRICE = 1000; |
There was a problem hiding this comment.
돈을 나타내는 숫자의 경우 천 단위를 구분하기 위해 "1_000"와 같이 언더바를 통해 구분할 수 있다.
| THREE(3, 5_000), | ||
| FOUR(4, 50_000), | ||
| FIVE(5, 1_500_000), | ||
| SIX(6, 2_000_000_000); |
There was a problem hiding this comment.
매칭 숫자보다 1등, 2등과 같은 표현의 이름을 사용하는 것은 어떨까?
| import java.util.List; | ||
| import java.util.Set; | ||
|
|
||
| public class WinningNumbers { |
There was a problem hiding this comment.
Lotto 객체와 다를 것이 없어 보인다.
중복으로 보이는데 Lotto를 사용하면 어떨까?
| @Test | ||
| @DisplayName("로또 번호는 반드시 6개여야 한다") | ||
| void lottoMustHaveSixNumbers() { | ||
| assertThatThrownBy(() -> new Lotto(List.of(1,2,3,4,5))) |
There was a problem hiding this comment.
| assertThatThrownBy(() -> new Lotto(List.of(1,2,3,4,5))) | |
| assertThatThrownBy(() -> new Lotto(List.of(1, 2, 3, 4, 5))) |
컨벤션 위반
| @Test | ||
| @DisplayName("로또 번호는 오름차순으로 정렬된다") | ||
| void lottoNumbersAreSorted() { | ||
| Lotto lotto = new Lotto(List.of(8, 21, 23, 41, 42, 43)); |
There was a problem hiding this comment.
| Lotto lotto = new Lotto(List.of(8, 21, 23, 41, 42, 43)); | |
| Lotto lotto = new Lotto(8, 21, 23, 41, 42, 43); |
가변 인자를 지원하는 생성자를 추가해 위와 같이 구현 가능하도록 지원하면 어떨까?
2 participants
개인 사정들로 인해 많이 미루어졌네요.. 남은 짧은 기간 최대한 보람찬 성과 남겨보도록 하겠습니다.