PR by opdshe · Pull Request #1 · OOP-study-media/java-lotto

opdshe · 2020-04-12T10:26:44Z

No description provided.

…dongheon

ghost · 2020-04-12T13:07:33Z

src/main/java/domain/Lotto.java

+        }
+    }
+
+    public void createNumbers() {


Lotto의 생성자에서만 사용하는 것 같은데, 접근제어자를 private으로 해야하지 않을까?
그리고 보통 private메서드는 클래스에서 사용되는 순서대로 위치시켜
public ex1() { ex2(); ex3(); }
private ex2() { ... }
private ex3() { ... }

ghost · 2020-04-12T13:09:38Z

src/main/java/domain/Lotto.java

+    }
+
+    public List<Integer> getNumbers() {
+        return numbers;


보통 클래스의 필드를 private으로 선언하고, get메서드를 이용하는 이유는 뭘까?
그리고, 여기서 문제가 될 만한 요소는 뭐가 있을지 한번 생각해봐

ghost · 2020-04-12T13:17:36Z

src/main/java/domain/WinningLotto.java

        this.bonusNo = bonusNo;
    }

+    public int countOfMatch(Lotto userlotto) {


파라미터 이름은 camel case 규칙을 지켜야돼.

ghost · 2020-04-12T13:18:26Z

src/main/java/domain/WinningLotto.java

+    public int countOfMatch(Lotto userlotto) {
+        int countOfMatch = 0;
+        for (Integer integer : userlotto.getNumbers()) {
+            if (lotto.getNumbers().contains(integer)) {


indent depth는 1까지만 허용되는 규칙이 있어.
userLotto에서 숫자를 꺼내지말고, Lotto 객체에 메시지를 보내도록 수정하면 어떨까?

ghost · 2020-04-12T13:19:59Z

src/main/java/domain/WinningLotto.java

+        return countOfMatch;
+    }
+
+    public boolean isContainsBounus(Lotto userLotto) {


is 와 contains의 의미가 중복돼.
containsBonusNumber가 좋을 것 같아.

ghost · 2020-04-12T13:24:40Z

src/main/java/lotto/App.java

+        do {
+            System.out.println("지난 주 당첨 번호를 입력해 주세요.");
+            answers = Arrays.stream(sc.nextLine().split(","))
+                    .map(String::trim).map(Integer::parseInt).distinct()


.마다 줄 바꿈을 해주면 가독성이 더 좋아져
10라인 맞추려고 이렇게 한거면, 보너스볼 입력을 따로 메서드로 분리시켜도 될 것 같은데?

ghost · 2020-04-12T13:25:43Z

src/main/java/lotto/App.java

+
+    public static void printLottos() {
+        System.out.println(purchasingAmount / LOTTO_PRICE + "개를 구매했습니다.");
+        for (Lotto lotto : lottos) {


lottos.foreach( ... ) 를 사용해봐

ghost · 2020-04-12T13:30:27Z

src/main/java/lotto/App.java

+
+    public static void initLottos() {
+        for (int i = 0; i < purchasingAmount / LOTTO_PRICE; i++) {
+            lottos.add(new Lotto(new ArrayList<>()));


로또 객체를 생성할 때, 빈 리스트가 아니라 숫자가 들어있는 리스트를 넘길 수는 없을까?

ghost · 2020-04-12T13:41:52Z

src/main/java/lotto/App.java

+        }
+    }
+
+    public static void printRankCount() {


이 메서드의 내용이 길어지는 이유는 당첨을 계산하는 로직과 출력하는 로직이 모두 포함되어있어서 인 것 같아.
각각 분리하면 어떨까?

ghost · 2020-04-16T09:05:52Z

src/main/java/lotto/App.java


-    public static void setAnswers() {
+    private static void initLottos() {
+        for (int i = 0; i < purchasingAmount / LOTTO_PRICE; i++) {


for 문 안에 i < purchasingAmount / LOTTO_PRICE 되어있으면 매번 돌때마다 i < purchasingAmount / LOTTO_PRICE를 계산해서 반복할지를 판단하게 되는데, 매번 계산할 필요가 있을까?

ghost · 2020-04-16T09:24:08Z

src/main/java/lotto/App.java

+        do {
+            System.out.println("지난 주 당첨 번호를 입력해 주세요.");
+            answers = Arrays.stream(sc.nextLine()
+                    .split(","))


Arrays.stream(sc.nextLine().split(",")) .map(String:trim) .map(Integer::parseInt)) .distinct() .collect(Collectors.toList());

첫줄에는 스트림으로 바꾸는 코드를 작성하고, 스트림 api별로 줄바꿈을 해주면 어떤 작업이 이뤄지는지 명확하게 보일 것 같아

ghost · 2020-04-16T09:33:03Z

src/main/java/lotto/App.java

+    private static void setBonusNum() {
+        do {
+            System.out.println("보너스 볼을 입력해 주세요.");
+        } while (!checkBonusNum(answers, bonusNum = sc.nextInt()));


do { System.out.println("보너스 볼을 입력해 주세요."); bonusNum = sc.nextInt(); } while (!checkBonusNum(answers, bonusNum));

이게 낫지않을까??

ghost · 2020-04-16T09:54:17Z

src/main/java/lotto/App.java

+    private static Map<Rank, Integer> result = new TreeMap<>();
+    private static int purchasingAmount;
+    private static int bonusNum;
+    private static int prizeMoney;


App클래스의 필드로 선언하는 변수의 갯수를 줄여볼 수 있을까?

opdshe added 16 commits April 11, 2020 13:05

initial commit

e928c46

Update README.md

82615c0

Update README.md

7dfc8ed

구입 금액 입력(검증포함)

96c2afa

Update README.md

fd2b224

Update README.md

be9a72e

랜덤변수 생성, 로또 리스트 초기화

a686b73

Update README.md

fb4640e

로또 리스트 출력 기능 추가

9d0f8a2

Update README.md

5f44ec3

위닝 로또 생성(검증)

1fb0a85

Merge branch 'dongheon' of https://github.com/opdshe/java-lotto into …

2f850bb

…dongheon

Update README.md

4b210b5

HashMap초기화 함수

7d331a6

Update README.md

1964259

Rank Count출력, 수익률 출력

0766a98

ghost reviewed Apr 12, 2020

View reviewed changes

modified version

7afd8be

ghost reviewed Apr 16, 2020

View reviewed changes

opdshe added 3 commits April 27, 2020 15:48

Add : Add images

0a25c67

Update README.md

12fe540

docs: README.md 수정

b500acb

Conversation

opdshe commented Apr 12, 2020

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!

1 participant

Comments