240617 #2116
240617 #2116
Conversation
|
|
||
| // 로또 번호 셀렉 | ||
| public ThrowableAssert.ThrowingCallable showNum(int money) throws MyException { | ||
| System.out.println(); |
| private List<Integer> selectNum() throws MyException { | ||
| // https://qh5944.tistory.com/152 참고 -, UnsupportedOperationException 해결 | ||
| List<Integer> numbers = Randoms.pickUniqueNumbersInRange(1, 45, 6); | ||
| List<Integer> result = new ArrayList<>(); |
There was a problem hiding this comment.
List result = new ArrayList<>(Randoms.pickUniqueNumbersInRange(1, 45, 6)); 이렇게 하면 더 좋을거 같아요
| } | ||
|
|
||
| // 수익률 구하기 | ||
| private double rate(HashMap<Integer, Integer> result, int n) { |
There was a problem hiding this comment.
하드코딩은 좋지 않은 것 같아요 😭
enum을 좀 더 활용했으면 좋았을 것 같아요
| @@ -1,7 +1,9 @@ | |||
| package lotto; | |||
|
|
|||
| public class Application { | |||
| public static void main(String[] args) { | |||
| public static void main(String[] args) throws MyException { | |||
There was a problem hiding this comment.
showMain 메소드에서는 예외를 던지지 않는데
메인에서 예외를 던지도록 되어 있네요
만약 메인에서 예외를 throws 한다면 어떻게 되나요?
| } | ||
|
|
||
| // 구입금액 입력받기 | ||
| public ThrowableAssert.ThrowingCallable getMoney() { |
There was a problem hiding this comment.
예외를 반환하는데 예외의 타입이 assertj를 사용하시는데
자바 기본의 Throwable를 사용하는건 어떨까요?
그리고 null만 리턴하고 있어서 맞추는게 좋을 것 같아요 😢
| } | ||
|
|
||
| // 1000으로 나누어 떨어지지 않으면 exception | ||
| public ThrowableAssert.ThrowingCallable getModulo(int money) throws MyException { |
There was a problem hiding this comment.
나누어 떨어지지 않으면 예외만 던지도록 하면 더 좋을 것 같아요
getModulo, showNum 메소드는 테스트 외에서 사용하지 않기 때문에
캡슐화를 하는게 어떨가 싶어요
테스트 때문에 public으로 변경하셨다면 어떻게 하면 좋을지 고민해보면 좋을 것 같아요
| @@ -0,0 +1,10 @@ | |||
| package lotto; | |||
| // https://staticclass.tistory.com/72 참고 | |||
| public class MyException extends java.lang.Exception { | |||
There was a problem hiding this comment.
커스텀 예외를 작성한 부분이 인상적이네요! 😃
Exception 패키지명 부분을 빼줬으면 더 좋았을 것 같아요
| public ThrowableAssert.ThrowingCallable showNum(int money) throws MyException { | ||
| System.out.println(); | ||
| System.out.println(money + "개를 구매했습니다."); | ||
| List<Integer>[] lotto = new List[money * 6]; |
There was a problem hiding this comment.
배열은 가급적 사용하지 않는 걸 추천해요
다른 Collection 객체로 변경하게 되면 많은 리팩토링이 발생할 수 있어요
그리고 Lotto 객체를 사용했다면 좋았지 않을까 싶어요
| Collections.sort(result); | ||
| } catch (NumberFormatException e) { | ||
| System.out.println("[ERROR] 숫자가 아닙니다."); | ||
| selectResult(); |
There was a problem hiding this comment.
순환 참조를 발생하고 toList 메소드를 실행하는 부분에 재귀를 해서 스택오버플로우 발생 위험이 있을 것 같아요
throw 예외를 발생하게 하는 건 어떨까요?
| // 5개+보너스숫자 확인 | ||
| private boolean countBonus(List<Integer> lotto, int bonus) { | ||
| for (Integer integer : lotto) { | ||
| if (integer == bonus) { |
There was a problem hiding this comment.
contains();contains를 사용했다면 코드도 줄고 가독성도 더 높아질 것 같아요
| } | ||
|
|
||
| // 수익률 구하기 | ||
| private double rate(HashMap<Integer, Integer> result, int n) { |
There was a problem hiding this comment.
하드코딩은 좋지 않은 것 같아요 😭
enum을 좀 더 활용했으면 좋았을 것 같아요
| private double rate(HashMap<Integer, Integer> result, int n) { | ||
| double avg = (double) (result.getOrDefault(3, 0) * 5 + result.getOrDefault(4, 0) * 50 | ||
| + result.getOrDefault(5, 0) * 1500 + result.getOrDefault(7, 0) * 30000 | ||
| + result.getOrDefault(6, 0) * 2000000) / n * 100; |
| public class MoneyTest { | ||
| @DisplayName("구입 금액 입력받기") | ||
| @Test | ||
| void nonNumericTest() throws MyException { |
There was a problem hiding this comment.
기능 구현한 하나의 메소드에서 많은 일을 처리하고 있어서 테스트하기 힘들었을 것 같아요
예외 처리에 대한 테스트도 좋지만 성공에 대한 케이스도 있으면 구현하는데 더 수월할 수 있어요
No description provided.