240617 by kimdohee58 · Pull Request #2116 · woowacourse-precourse/java-lotto-6

kimdohee58 · 2024-06-16T19:38:52Z

No description provided.

chenghaLim · 2024-06-17T12:22:28Z

src/main/java/lotto/Money.java

+
+    // 로또 번호 셀렉
+    public ThrowableAssert.ThrowingCallable showNum(int money) throws MyException {
+        System.out.println();


따로 출력되는 부분을 나누면 더 좋을 거 같아요~

chenghaLim · 2024-06-17T12:24:01Z

src/main/java/lotto/Money.java

+    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<>();


List result = new ArrayList<>(Randoms.pickUniqueNumbersInRange(1, 45, 6)); 이렇게 하면 더 좋을거 같아요

chenghaLim · 2024-06-17T12:26:18Z

src/main/java/lotto/Result.java

+    }
+
+    // 수익률 구하기
+    private double rate(HashMap<Integer, Integer> result, int n) {


수익률의 금액도 같이 이넘에 넣으면 더 보기 좋을 거 같아요~

하드코딩은 좋지 않은 것 같아요 😭
enum을 좀 더 활용했으면 좋았을 것 같아요

xxxzerone · 2024-06-17T12:13:35Z

src/main/java/lotto/Application.java


 public class Application {
-    public static void main(String[] args) {
+    public static void main(String[] args) throws MyException {


showMain 메소드에서는 예외를 던지지 않는데
메인에서 예외를 던지도록 되어 있네요
만약 메인에서 예외를 throws 한다면 어떻게 되나요?

xxxzerone · 2024-06-17T12:16:52Z

src/main/java/lotto/Money.java

+    }
+
+    // 구입금액 입력받기
+    public ThrowableAssert.ThrowingCallable getMoney() {


예외를 반환하는데 예외의 타입이 assertj를 사용하시는데
자바 기본의 Throwable를 사용하는건 어떨까요?
그리고 null만 리턴하고 있어서 맞추는게 좋을 것 같아요 😢

xxxzerone · 2024-06-17T14:41:51Z

src/main/java/lotto/Money.java

+    }
+
+    // 1000으로 나누어 떨어지지 않으면 exception
+    public ThrowableAssert.ThrowingCallable getModulo(int money) throws MyException {


나누어 떨어지지 않으면 예외만 던지도록 하면 더 좋을 것 같아요
getModulo, showNum 메소드는 테스트 외에서 사용하지 않기 때문에
캡슐화를 하는게 어떨가 싶어요
테스트 때문에 public으로 변경하셨다면 어떻게 하면 좋을지 고민해보면 좋을 것 같아요

xxxzerone · 2024-06-17T14:53:49Z

src/main/java/lotto/MyException.java

@@ -0,0 +1,10 @@
+package lotto;
+// https://staticclass.tistory.com/72 참고
+public class MyException extends java.lang.Exception {


커스텀 예외를 작성한 부분이 인상적이네요! 😃
Exception 패키지명 부분을 빼줬으면 더 좋았을 것 같아요

xxxzerone · 2024-06-17T15:07:29Z

src/main/java/lotto/Money.java

+    public ThrowableAssert.ThrowingCallable showNum(int money) throws MyException {
+        System.out.println();
+        System.out.println(money + "개를 구매했습니다.");
+        List<Integer>[] lotto = new List[money * 6];


배열은 가급적 사용하지 않는 걸 추천해요
다른 Collection 객체로 변경하게 되면 많은 리팩토링이 발생할 수 있어요

그리고 Lotto 객체를 사용했다면 좋았지 않을까 싶어요

xxxzerone · 2024-06-17T15:16:03Z

src/main/java/lotto/WinNum.java

+            Collections.sort(result);
+        } catch (NumberFormatException e) {
+            System.out.println("[ERROR] 숫자가 아닙니다.");
+            selectResult();


순환 참조를 발생하고 toList 메소드를 실행하는 부분에 재귀를 해서 스택오버플로우 발생 위험이 있을 것 같아요
throw 예외를 발생하게 하는 건 어떨까요?

xxxzerone · 2024-06-17T15:20:26Z

src/main/java/lotto/Result.java

+    // 5개+보너스숫자 확인
+    private boolean countBonus(List<Integer> lotto, int bonus) {
+        for (Integer integer : lotto) {
+            if (integer == bonus) {


contains();

contains를 사용했다면 코드도 줄고 가독성도 더 높아질 것 같아요

xxxzerone · 2024-06-17T15:23:49Z

src/main/java/lotto/Result.java

+    }
+
+    // 수익률 구하기
+    private double rate(HashMap<Integer, Integer> result, int n) {


하드코딩은 좋지 않은 것 같아요 😭
enum을 좀 더 활용했으면 좋았을 것 같아요

xxxzerone · 2024-06-17T15:24:49Z

src/main/java/lotto/Result.java

+    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;


소수점 둘째 자리에서 반올림이 됐는지 확인이 어렵네요

xxxzerone · 2024-06-17T15:28:01Z

src/test/java/lotto/MoneyTest.java

+public class MoneyTest {
+    @DisplayName("구입 금액 입력받기")
+    @Test
+    void nonNumericTest() throws MyException {


기능 구현한 하나의 메소드에서 많은 일을 처리하고 있어서 테스트하기 힘들었을 것 같아요
예외 처리에 대한 테스트도 좋지만 성공에 대한 케이스도 있으면 구현하는데 더 수월할 수 있어요

240617

64b96ae

chenghaLim reviewed Jun 17, 2024

View reviewed changes

xxxzerone reviewed Jun 17, 2024

View reviewed changes

Comments

Conversation

kimdohee58 commented Jun 16, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants