PR#1
Conversation
| } | ||
| } | ||
|
|
||
| public void createNumbers() { |
There was a problem hiding this comment.
Lotto의 생성자에서만 사용하는 것 같은데, 접근제어자를 private으로 해야하지 않을까?
그리고 보통 private메서드는 클래스에서 사용되는 순서대로 위치시켜
public ex1() { ex2(); ex3(); }
private ex2() { ... }
private ex3() { ... }
| } | ||
|
|
||
| public List<Integer> getNumbers() { | ||
| return numbers; |
There was a problem hiding this comment.
보통 클래스의 필드를 private으로 선언하고, get메서드를 이용하는 이유는 뭘까?
그리고, 여기서 문제가 될 만한 요소는 뭐가 있을지 한번 생각해봐
| this.bonusNo = bonusNo; | ||
| } | ||
|
|
||
| public int countOfMatch(Lotto userlotto) { |
| public int countOfMatch(Lotto userlotto) { | ||
| int countOfMatch = 0; | ||
| for (Integer integer : userlotto.getNumbers()) { | ||
| if (lotto.getNumbers().contains(integer)) { |
There was a problem hiding this comment.
indent depth는 1까지만 허용되는 규칙이 있어.
userLotto에서 숫자를 꺼내지말고, Lotto 객체에 메시지를 보내도록 수정하면 어떨까?
| return countOfMatch; | ||
| } | ||
|
|
||
| public boolean isContainsBounus(Lotto userLotto) { |
There was a problem hiding this comment.
is 와 contains의 의미가 중복돼.
containsBonusNumber가 좋을 것 같아.
| do { | ||
| System.out.println("지난 주 당첨 번호를 입력해 주세요."); | ||
| answers = Arrays.stream(sc.nextLine().split(",")) | ||
| .map(String::trim).map(Integer::parseInt).distinct() |
There was a problem hiding this comment.
.마다 줄 바꿈을 해주면 가독성이 더 좋아져
10라인 맞추려고 이렇게 한거면, 보너스볼 입력을 따로 메서드로 분리시켜도 될 것 같은데?
|
|
||
| public static void printLottos() { | ||
| System.out.println(purchasingAmount / LOTTO_PRICE + "개를 구매했습니다."); | ||
| for (Lotto lotto : lottos) { |
|
|
||
| public static void initLottos() { | ||
| for (int i = 0; i < purchasingAmount / LOTTO_PRICE; i++) { | ||
| lottos.add(new Lotto(new ArrayList<>())); |
There was a problem hiding this comment.
로또 객체를 생성할 때, 빈 리스트가 아니라 숫자가 들어있는 리스트를 넘길 수는 없을까?
| } | ||
| } | ||
|
|
||
| public static void printRankCount() { |
There was a problem hiding this comment.
이 메서드의 내용이 길어지는 이유는 당첨을 계산하는 로직과 출력하는 로직이 모두 포함되어있어서 인 것 같아.
각각 분리하면 어떨까?
|
|
||
| public static void setAnswers() { | ||
| private static void initLottos() { | ||
| for (int i = 0; i < purchasingAmount / LOTTO_PRICE; i++) { |
There was a problem hiding this comment.
for 문 안에 i < purchasingAmount / LOTTO_PRICE 되어있으면 매번 돌때마다 i < purchasingAmount / LOTTO_PRICE를 계산해서 반복할지를 판단하게 되는데, 매번 계산할 필요가 있을까?
| do { | ||
| System.out.println("지난 주 당첨 번호를 입력해 주세요."); | ||
| answers = Arrays.stream(sc.nextLine() | ||
| .split(",")) |
There was a problem hiding this comment.
Arrays.stream(sc.nextLine().split(","))
.map(String:trim)
.map(Integer::parseInt))
.distinct()
.collect(Collectors.toList());
첫줄에는 스트림으로 바꾸는 코드를 작성하고, 스트림 api별로 줄바꿈을 해주면 어떤 작업이 이뤄지는지 명확하게 보일 것 같아
| private static void setBonusNum() { | ||
| do { | ||
| System.out.println("보너스 볼을 입력해 주세요."); | ||
| } while (!checkBonusNum(answers, bonusNum = sc.nextInt())); |
There was a problem hiding this comment.
do {
System.out.println("보너스 볼을 입력해 주세요.");
bonusNum = sc.nextInt();
} while (!checkBonusNum(answers, bonusNum));
이게 낫지않을까??
| private static Map<Rank, Integer> result = new TreeMap<>(); | ||
| private static int purchasingAmount; | ||
| private static int bonusNum; | ||
| private static int prizeMoney; |
1 participant
No description provided.