Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Step2: 로또(자동) #3964

Open
wants to merge 8 commits into
base: maeng24
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 50 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,48 @@
# STEP 2: 로또(자동)

## 기능 요구사항
* 로또 구입 금액을 입력하면 구입 금액에 해당하는 로또를 발급해야 한다.
* 로또 1장의 가격은 1000원이다.

## 진행 목록
- [x] 로또가 6개의 랜덤 숫자를 가지도록 한다.
- [x] 로또 생성중에 겹치는 숫자가 있거나, 숫자가 6개가 아닌 경우 exception을 발생시킨다.
- [x] 1개의 당첨숫자를 입력받아 가지도록 한다.
- [x] 입력받은 당첨숫자와 n개의 숫자가 일치하는 로또의 수를 확인한다.
- [x] 사용자에게 입력받은 금액으로 로또를 발급한다.
-[x] 로또의 수익률을 계산한다.
-[x] 로또의 결과를 출력한다.

## 힌트
- 로또 자동 생성은 Collections.shuffle() 메소드 활용한다.
- Collections.sort() 메소드를 활용해 정렬 가능하다.
- ArrayList의 contains() 메소드를 활용하면 어떤 값이 존재하는지 유무를 판단할 수 있다.

## 프로그래밍 요구사항
- 모든 기능을 TDD로 구현해 단위 테스트가 존재해야 한다. 단, UI(System.out, System.in) 로직은 제외
- 핵심 로직을 구현하는 코드와 UI를 담당하는 로직을 구분한다.
- UI 로직을 InputView, ResultView와 같은 클래스를 추가해 분리한다.
- indent(인덴트, 들여쓰기) depth를 2를 넘지 않도록 구현한다. 1까지만 허용한다.
- 예를 들어 while문 안에 if문이 있으면 들여쓰기는 2이다.
- 힌트: indent(인덴트, 들여쓰기) depth를 줄이는 좋은 방법은 함수(또는 메소드)를 분리하면 된다.
- 함수(또는 메소드)의 길이가 15라인을 넘어가지 않도록 구현한다.
- 함수(또는 메소드)가 한 가지 일만 잘 하도록 구현한다.
- 모든 로직에 단위 테스트를 구현한다. 단, UI(System.out, System.in) 로직은 제외
- 핵심 로직을 구현하는 코드와 UI를 담당하는 로직을 구분한다.
- UI 로직을 InputView, ResultView와 같은 클래스를 추가해 분리한다.
- 자바 코드 컨벤션을 지키면서 프로그래밍한다.
- 참고문서: https://google.github.io/styleguide/javaguide.html 또는 https://myeonguni.tistory.com/1596
- else 예약어를 쓰지 않는다.
- 힌트: if 조건절에서 값을 return하는 방식으로 구현하면 else를 사용하지 않아도 된다.
- else를 쓰지 말라고 하니 switch/case로 구현하는 경우가 있는데 switch/case도 허용하지 않는다.

## 기능 목록 및 commit 로그 요구사항
- 기능을 구현하기 전에 README.md 파일에 구현할 기능 목록을 정리해 추가한다.
- git의 commit 단위는 앞 단계에서 README.md 파일에 정리한 기능 목록 단위로 추가한다.

-----------------------------------------------
# STEP 1: 문자열 사칙 연산 계산기

# 문자열 사칙 연산 계산기 구현
| 이번 미션의 핵심은 내가 구현하는 코드에 단위 테스트를 추가하는 경험을 하는 것이다.
| 모든 예외 상황을 처리하기 위해 너무 복잡하게 접근하지 않아도 된다.
Expand All @@ -13,11 +58,11 @@
-[x] 사용자에게 입력을 받을 수 있는 input view를 구현한다.

## 기능 요구사항
사용자가 입력한 문자열 값에 따라 사칙연산을 수행할 수 있는 계산기를 구현해야 한다.
입력 문자열의 숫자와 사칙 연산 사이에는 반드시 빈 공백 문자열이 있다고 가정한다.
나눗셈의 경우 결과 값을 정수로 떨어지는 값으로 한정한다.
문자열 계산기는 사칙연산의 계산 우선순위가 아닌 입력 값에 따라 계산 순서가 결정된다. 즉, 수학에서는 곱셈, 나눗셈이 덧셈, 뺄셈 보다 먼저 계산해야 하지만 이를 무시한다.
예를 들어 2 + 3 * 4 / 2와 같은 문자열을 입력할 경우 2 + 3 * 4 / 2 실행 결과인 10을 출력해야 한다.
* 사용자가 입력한 문자열 값에 따라 사칙연산을 수행할 수 있는 계산기를 구현해야 한다.
* 입력 문자열의 숫자와 사칙 연산 사이에는 반드시 빈 공백 문자열이 있다고 가정한다.
* 나눗셈의 경우 결과 값을 정수로 떨어지는 값으로 한정한다.
* 문자열 계산기는 사칙연산의 계산 우선순위가 아닌 입력 값에 따라 계산 순서가 결정된다. 즉, 수학에서는 곱셈, 나눗셈이 덧셈, 뺄셈 보다 먼저 계산해야 하지만 이를 무시한다.
* 예를 들어 2 + 3 * 4 / 2와 같은 문자열을 입력할 경우 2 + 3 * 4 / 2 실행 결과인 10을 출력해야 한다.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

요구사항 정리도 잘 해주셨고, 이전단계 개선사항도 잘 정리 해주셨네요 💯


## 프로그래밍 요구사항
- indent(들여쓰기) depth를 2단계에서 1단계로 줄여라.
Expand Down
20 changes: 20 additions & 0 deletions src/main/java/Main.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import lotto.domain.Lotto;
import lotto.domain.LottoChecker;
import lotto.view.LottoInputView;
import lotto.view.LottoOutputView;

import java.util.List;

public class Main {
public static void main(String[] args) {

LottoInputView.initiateScanner();
List<Lotto> purchasedLottos = LottoInputView.createPurchasedLottos(LottoInputView.scanAmount());

LottoOutputView.printPurchaseInfo(purchasedLottos);

Lotto winLotto = LottoInputView.createLottoWithScan(LottoInputView.scanWinNumbers());

LottoOutputView.printResultInfo(new LottoChecker(purchasedLottos, winLotto));
}
}
38 changes: 38 additions & 0 deletions src/main/java/lotto/domain/Lotto.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package lotto.domain;

import java.util.Collections;
import java.util.List;

public class Lotto {
private final List<Integer> numbers;

public Lotto(List<Integer> numbers) {
numberEmptyCheck(numbers);
duplicateNumberCheck(numbers);

Collections.sort(numbers);
this.numbers = numbers;
}

public Integer getLottoSize() {
return this.numbers.size();
}

private void duplicateNumberCheck(List<Integer> numbers) {
if (numbers.stream().distinct().count() < 6)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

소소한 의견인데 Set을 활용해볼 수도 있을 것 같습니다 🙇

throw new IllegalArgumentException("중복된 숫자가 있습니다.");
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

당첨번호도 Lotto객체와 동일한 속성을 가지고 있습니다.
같이 검증되면 더 좋을 것 같습니다 😄

image


private void numberEmptyCheck(List<Integer> numbers) {
if (numbers == null || numbers.isEmpty())
throw new IllegalArgumentException("로또 번호가 없습니다.");
}

public Boolean containsNumbers(List<Integer> numbers) {
return this.numbers.containsAll(numbers);
}

public Integer getNumberByIndex(Integer index) {
return this.numbers.get(index);
}
}
58 changes: 58 additions & 0 deletions src/main/java/lotto/domain/LottoChecker.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
package lotto.domain;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;

public class LottoChecker {

private final int[] matchCounts;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

array를 사용하시면 계속적으로 indexing하는 코드를 사용할 수 밖에 없습니다.
(for (int i = 0 ~ 같은) List같은 컬렉션을 활용해보시는건 어떠실까요?? 😄
물론 array를 사용하면 detail한 제어가 가능하긴 합니다만 매번 종료 index및 array index를 체크해야 하고, 코드 읽기도 좀 복잡한 면이 있습니다 🙇

private final List<Lotto> purchasedLottos;
private final Lotto winNumberLotto;

public LottoChecker(List<Lotto> purchasedLottos, Lotto winNumberLotto) {
winnerLottoEmptyCheck(winNumberLotto);

this.purchasedLottos = purchasedLottos != null ? purchasedLottos : new ArrayList<>();
this.winNumberLotto = winNumberLotto;
int[] matchCounts = new int[7];

for (Lotto lotto : this.purchasedLottos) {
matchCounts[countMatch(lotto)]++;
}

this.matchCounts = matchCounts;
}

private int countMatch(Lotto lotto) {
int count = 0;
for (int i = 0; i < 6; i++) {
count += countMatchNumber(lotto.getNumberByIndex(i));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lotto.getNumberByIndex(i) 매번 index 값인 i를 통해서 값을 가져와야 하는 구조입니다 😄

get을 사용하지 말라는 요구사항 때문에 이렇게 작업해주신 것 같은데,
제 개인적인 생각으로는 get을 아예 사용안하는 건 불가능 하다고 생각합니다.
사용하더라도 안전한 방식으로 사용한다면 크게 문제는 없다는 입장인데요,
get을 사용하되 numbers를 불변객체, 혹은 복제하여 반환한다면 (원본에 이상이 없도록) 괜찮다는 입장입니다.

// Lotto
public List<Integer> getNumbers() {
        return Collections.unmodifiableList(this.numbers);
}

// AS IS 
for (int i = 0; i < 6; i++) {
      count += countMatchNumber(lotto.getNumberByIndex(i));
 }

// TO BE 
for (Integer number : lotto.getNumbers()) {
      count += countMatchNumber(number);
 }

}
return count;
}

private void winnerLottoEmptyCheck(Lotto winnerLotto) {
if (winnerLotto == null)
throw new IllegalArgumentException("우승 번호가 없습니다.");
}

public Boolean containsWinNumbers(List<Integer> numbers) {
return winNumberLotto.containsNumbers(numbers);
}

public Integer countMatchNumber(Integer number) {
if (containsWinNumbers(Arrays.asList(number)))
return 1;
return 0;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분 처음에 코드 읽기가 좀 어려운 부분이었습니다 🤔
winNumberLottoLottonumber를 주고 match되었는지를 물어보면 어떨까요??
Lotto에서 number를 받아 1, 0 값을 반환하는 method를 제공하는게 더 좋을 것 같습니다. 그리고 번호를 관리하는 주체도 Lotto가 맞으니까요 😄


public Integer getWinnerCount(int index) {
return this.matchCounts[index];
}

public Integer getPurchaseCount() {
return this.purchasedLottos.size();
}

}
28 changes: 28 additions & 0 deletions src/main/java/lotto/domain/LottoFactory.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package lotto.domain;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Random;
import java.util.stream.Collectors;
import java.util.stream.IntStream;

public class LottoFactory {

private static final List<Integer> numbers =
IntStream.rangeClosed(1, 45)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1, 45도 매직넘버이니 공통 변수로 추출되면 좋을 것 같습니다 🙇

.boxed()
.collect(Collectors.toList());

private static final Random random = new Random();

public static Lotto createLotto(List<Integer> lottoNumbers) {
return new Lotto(lottoNumbers);
}

public static Lotto createLottoWithRandomNumbers() {
List<Integer> copyOfNumbers = new ArrayList<>(numbers);
Collections.shuffle(copyOfNumbers, random);
return createLotto(copyOfNumbers.subList(0, 6));
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

로또 생성 때 적용되는 번호 뿐만 아닐, 당첨 번호도 동일한 번호 (1~45) 검증이 필요할 것 같습니다 🙇
image

}
23 changes: 23 additions & 0 deletions src/main/java/lotto/domain/LottoPrizeCalculator.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package lotto.domain;

public class LottoPrizeCalculator {

private static final Integer LOTTO_PRICE = 1000;
private static final int[] prizeMoney = {0, 0, 0, 5000, 50000, 1500000, 2000000000};

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0, 0, 0, 5000, 50000, 1500000, 2000000000은 구조 변경을 통해 가독성 개선이 필요할 것 같습니다 😄

public static Double calculateRateOfReturn(LottoChecker lottoChecker) {
int prize = 0;
for (int i = 3; i < 7; i++) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

위에서도 의견 드렸고, LottoOutputView.printResultInfo에도 의견 드렸지만, 이 부분은 구조 개선이 좀 필요할 것 같습니다 😄
3개 ~ 6개 당첨 그리고 그 금액에 대한 정보를 가지고 있는 Enum이나 별도의 객체를 통해 관리 되도록 해보시는 건 어떨까요??
변하지 않는 정보이고 상수입니다.

1단계에서도 Enum을 활용해서 멋지게 구현을 해주셨는데요 👍
이 부분이야 말로 Enum이 적용되어야 할 부분이라 생각됩니다.

prize += calculatePrizeByNumberAndCount(i, lottoChecker.getWinnerCount(i));
}
return (double) prize / (lottoChecker.getPurchaseCount() * LOTTO_PRICE);
}

public static Integer calculatePrizeByNumberAndCount(int numberOfMatch, int count) {
return count * prizeMoney[numberOfMatch];
}

public static Integer getPrizeByIndex(int index) {
return prizeMoney[index];
}
}
54 changes: 54 additions & 0 deletions src/main/java/lotto/view/LottoInputView.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package lotto.view;

import lotto.domain.Lotto;
import lotto.domain.LottoFactory;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Scanner;
import java.util.stream.Collectors;

public class LottoInputView {

private static Scanner scanner;
private static final Integer LOTTO_PRICE = 1000;

public static void initiateScanner() {
scanner = new Scanner(System.in);
}

public static Integer scanAmount() {
System.out.println("구입금액을 입력해 주세요.");
Integer amount = scanner.nextInt();
scanner.nextLine();

return amount;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

아마 별도로 질문 주신 부분일 것 같습니다.

LOTTO_PRICE 값이 지금 여기에서도 사용되고 있고 LottoPrizeCalculator에도 사용되고 있는데요 🤔

보통 이럴 경우 2가지 해법이 있을 수 있을 것 같습니다.
공통으로 사용하는 상수로 추출하는 법이 있을 것 같습니다.
일종의 LottoConstants 같은 클래스를 만들어서 정적상수로 호출하고
여기저기 클래스에서 사용하는 방법이 있을 것 같아요 😄
하지만 그렇게 했을 경우, 위의 상황처럼 사용하는 곳마다 예외를 처리하는 코드가
추가되어야 하기 때문에 중복 코드가 발생할 확률이 높습니다.

다른 방법으로는 금액에 대한 원시객체를 만드는 방법입니다.
LottoPriceAmount나 혹은 LottoMoney같은 객체를 만들고 (네이밍은 적당한걸로 하시면 될 것 같아요 🙇)
해당 객체를 생성될 때 검증을 수행하게 하고, 그 뒤로는 해당 객체가
로또 금액과 관련된 모든일을 하도록 역할과 책임을 부여하는 방법이 있을 것 같아요 😄

저는 개인적으로 상황에 따라서 1, 2번을 선택적, 복합적으로 사용합니다.
이번에는 원시 객체로 포장되는게 좋을 것 같네요 😄

요 내용이 도움 되실 것 같습니다 🙇

JAVA 원시 값 포장
https://velog.io/@kanamycine/Java-%EC%9B%90%EC%8B%9C%EA%B0%92-%ED%8F%AC%EC%9E%A5


public static String scanWinNumbers() {
System.out.println("지난 주 당첨 번호를 입력해 주세요.");
return scanner.nextLine();
}

public static Lotto createLottoWithScan(String numberString) {
String[] numbers = numberString.split(", ");

return LottoFactory.createLotto(
Arrays.stream(numbers)
.map(Integer::parseInt)
.collect(Collectors.toList()));
}

public static List<Lotto> createPurchasedLottos(Integer amount) {
List<Lotto> purchasedLottos = new ArrayList<>();
int count = amount / LOTTO_PRICE;

for (int i = 0; i < count; i++) {
purchasedLottos.add(
LottoFactory.createLottoWithRandomNumbers());
}

return purchasedLottos;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LottoInputView가 로또를 생성하는 책임까지 가져가는 건 좀 과한 것 같습니다.
다른 도메인 객체나 혹은 서비스코드에서 로또를 생성하고 싶을 때는 LottoInputView를 아예 사용안하거나
console 입력이 아닌 다른 입력을 수단으로 생각하고
Lotto를 사고 싶다면 이 코드를 복붙해서 사용해야 하기 때문에 계속적으로 중복 될 것 같아요 🤔 (createLottoWithScan도 마찬가지 일 것 같습니다.)

LottoInputView는 사용자로부터 입력이 잘 되었는지, 값이 비어있지는 않은지 정도만 확인하고 Main에서 다른 객체를 통해 생성되도록 하는게 더 좋을 것 같습니다 😄

프로그래밍 요구사항
핵심 로직을 구현하는 코드와 UI를 담당하는 로직을 구분한다.

}
39 changes: 39 additions & 0 deletions src/main/java/lotto/view/LottoOutputView.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package lotto.view;

import lotto.domain.Lotto;
import lotto.domain.LottoChecker;
import lotto.domain.LottoPrizeCalculator;

import java.util.List;

public class LottoOutputView {

public static void printPurchaseInfo(List<Lotto> purchaseList) {
System.out.println(purchaseList.size() + "개를 구매했습니다.");
for (Lotto lotto : purchaseList) {
System.out.println("[" + getPurchaseInfoToString(lotto) + "]");
}
System.out.println();
}

private static String getPurchaseInfoToString(Lotto lotto) {
StringBuilder sb = new StringBuilder();
sb.append(lotto.getNumberByIndex(0).toString());

for (int i = 1; i < 6; i++) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1, 6 등의 매직넘버는 상수로 추출하시죠 😄

sb.append(", " + lotto.getNumberByIndex(i).toString());
}

return sb.toString();
}

public static void printResultInfo(LottoChecker lottoChecker) {
System.out.println();
System.out.println("당첨 통계");
System.out.println("---------");
for (int i = 3; i < 7; i++) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3, 7도 매직넘버 입니다. 지수님과 저야 요구사항을 다 알고 있기 때문에 3, 7이 의미하는 값이 어떤지 알지만, 아마 이 부분을 처음 보게될 다른 동료개발자들은 이해하려면 전체 코드 분석을 다 해야 하기 때문에 많은 어려움을 겪을 것 같습니다.
이 부분은 가독성 개선이 필요할 것 같아요.

System.out.println(i + "개 일치 (" + LottoPrizeCalculator.getPrizeByIndex(i) + "원)-" + lottoChecker.getWinnerCount(i) + "개");
}
System.out.println("총 수익률은 " + LottoPrizeCalculator.calculateRateOfReturn(lottoChecker) + "입니다.");
}
}
14 changes: 7 additions & 7 deletions src/main/java/stringCalculator/domain/MathExpression.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@

public class MathExpression {
private final Queue<Integer> numbers;
private final Queue<String> operations;
private final Queue<OperationEnum> operations;

MathExpression(Queue<Integer> numbers, Queue<String> operations) {
MathExpression(Queue<Integer> numbers, Queue<OperationEnum> operations) {
this.numbers = numbers;
this.operations = operations;
}
Expand All @@ -16,23 +16,23 @@ public Boolean numbersContains(List<Integer> contains) {
return numbers.containsAll(contains);
}

public Boolean operationsContains(List<String> contains) {
public Boolean operationsContains(List<OperationEnum> contains) {
return operations.containsAll(contains);
}

public Integer numberPoll(){
public Integer numberPoll() {
return numbers.poll();
}

public String operationPoll(){
public OperationEnum operationPoll() {
return operations.poll();
}

public Boolean numberHasNext(){
public Boolean numberHasNext() {
return numbers.iterator().hasNext();
}

public Boolean operationHasNext(){
public Boolean operationHasNext() {
return operations.iterator().hasNext();
}
}
Loading