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

Final #2101

Open
wants to merge 30 commits into
base: main
Choose a base branch
from
Open

Final #2101

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
da390e5
docs : 필요 기능 설계
zangsu Nov 23, 2023
533fc74
feat : Lotto가 사용할 리스트 타입을 `LottoNumber`를 사용하도록 변경
zangsu Nov 23, 2023
b734965
feat : LottoAnswer 추가
zangsu Nov 23, 2023
6d42b5b
feat : LottoGenerator, RandomLottoGenerator 추가
zangsu Nov 23, 2023
925a8c0
feat : LottoGenerator 추상 클래스로 변경 & 로또 생성 로직 추가
zangsu Nov 23, 2023
932a4f6
fix : Lotto 숫자 범위 수정, 테스트 추가
zangsu Nov 23, 2023
b45b762
feat : Lotto 비교 기능 추가
zangsu Nov 23, 2023
9df4d82
feat : Lotto 당첨 결과 구현
zangsu Nov 23, 2023
fa7d4ac
feat : LottoResult에 당첨금 정보 추가
zangsu Nov 23, 2023
14cca9a
feat : Reader에서 정수, 정수 리스트 입력 구현
zangsu Nov 23, 2023
fdae2c3
feat : 금액 입력 구현
zangsu Nov 23, 2023
9baf62c
feat : 재입력 예외 핸들러 구현
zangsu Nov 23, 2023
d846b8d
feat : Lottos 일급 컬렉션 사용
zangsu Nov 23, 2023
a613f47
feat : Lotto 구매 결과 출력
zangsu Nov 23, 2023
a46770f
style : 출력 형식 수정
zangsu Nov 23, 2023
b1b49f0
feat : 로또 당첨 결과들을 모아두는 `LottoResults` 생성
zangsu Nov 23, 2023
944b36e
feat : 로또 당첨 결과 출력
zangsu Nov 23, 2023
d9efd9d
feat : 수익률 출력
zangsu Nov 23, 2023
ff9e570
feat : 예외 처리 추출
zangsu Nov 23, 2023
e6c1959
feat : 금액 값 래핑 클래스 Money, Cash 생성
zangsu Nov 23, 2023
5e0acd2
refactor : 금액 패키지 생성
zangsu Nov 23, 2023
b9d23a4
refactor : 로또 숫자 범위 상수 추출
zangsu Nov 23, 2023
7b91f9b
refactor : LottoResultDTO 사용
zangsu Nov 23, 2023
4d22078
refactor : `private final` 추가
zangsu Nov 23, 2023
037f766
docs : 기능 구현 목록 정리
zangsu Nov 23, 2023
1c43fc2
refactor : 상수 추출
zangsu Nov 23, 2023
05d1e96
refactor : 출력에서 toString 사용 안하도록 리팩토링
zangsu Nov 23, 2023
2ce0bbe
refactor : ExceptionHandler에서 Printer 사용하기
zangsu Nov 23, 2023
90585eb
refactor : 0이 아닌 값을 가지는 `Money`를 위한 정적 팩토리 메서드 구현
zangsu Nov 23, 2023
ab861a9
refactor : 테스트 이름 변경
zangsu Nov 23, 2023
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
20 changes: 20 additions & 0 deletions docs/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
기능 구현 목록

## 기능 구현 목록

기능 목록

- 로또를 구매 한다
- 구입 금액을 입력
- 음수, 0, 또는 1,000으로 나누어 떨어지지 않는 값을 입력하면 예외 발생
- 구매한 로또를 출력함
- 당첨 번호를 입력한다.
- 당첨 번호는 6자리를, 보너스 번호는 1자리를 입력
- 당첨 번호가 서로 중복될 경우 예외 발생
- 보너스 번호가 당첨 번호와 중복될 경우 예외 발생
- 로또 숫자들이 정해진 범위 (1 ~ 45)를 벗어나면 예외 발생
- 결과를 출력한다.
- 당첨 통계
- n개 일치 (%d원) - %d개
- 수익률
- 총 수익률은 %f원 입니다.
3 changes: 2 additions & 1 deletion src/main/java/lotto/Application.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

public class Application {
public static void main(String[] args) {
// TODO: 프로그램 구현
LottoController lottoController = new LottoController();
lottoController.run();
}
}
20 changes: 0 additions & 20 deletions src/main/java/lotto/Lotto.java

This file was deleted.

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

import java.math.BigDecimal;
import java.util.List;
import lotto.domain.lotto.LottoService;
import lotto.domain.lotto.entity.Lotto;
import lotto.domain.lotto.entity.LottoAnswer;
import lotto.domain.lotto.entity.LottoResultCount;
import lotto.domain.lotto.entity.Lottos;
import lotto.domain.lotto.generator.RandomLottoGenerator;
import lotto.domain.lotto.money.Money;
import lotto.exception.RetryExceptionHandler;
import lotto.view.InputView;
import lotto.view.OutputView;

public class LottoController {
public static final BigDecimal PERCENT_DECIMAL = new BigDecimal(100);
private final InputView inputView = new InputView();
private final OutputView outputView = new OutputView();
private final RetryExceptionHandler handler = new RetryExceptionHandler();
private final LottoService lottoService = new LottoService(new RandomLottoGenerator());

void run() {
//로또 구입
Money purchaseMoney = getPurchaseMoney();
Lottos lottos = purchaseLotto(purchaseMoney);
printPurchasedLotto(lottos);

//정답 생성
LottoAnswer answer = getLottoAnswer();

//결과 출력
LottoResultCount results = lottos.getResults(answer);
printResult(results);
printRevenu(purchaseMoney, results);
}
Comment on lines +23 to +36
Copy link

Choose a reason for hiding this comment

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

👍👍


private void printRevenu(Money purchaseMoney, LottoResultCount results) {
BigDecimal initMoney = purchaseMoney.toBigDecimal();
BigDecimal totalPrize = results.getTotalPrize();
outputView.printRevenue(totalPrize.divide(initMoney).multiply(PERCENT_DECIMAL));
}

private void printResult(LottoResultCount results) {
outputView.printResults(results);
}

private Money getPurchaseMoney() {
return handler.get(() -> {
int purchaseMoney = inputView.getPurchaseMoney();
return Money.nonZeroMoney(purchaseMoney);
});

}

private Lottos purchaseLotto(Money purchaseMoney) {
return lottoService.purchaseLottos(purchaseMoney);
}

private LottoAnswer getLottoAnswer() {
Lotto lotto = getLottoUsingInput();
return generateLottoAnswer(lotto);
}

private LottoAnswer generateLottoAnswer(Lotto lotto) {
return handler.get(() -> {
int bonusNumber = inputView.getBonusNumber();
return new LottoAnswer(lotto, bonusNumber);
}
);
}

private Lotto getLottoUsingInput() {
return handler.get(() -> {
List<Integer> lottoNumbers = inputView.getLottoNumbers();
return new Lotto(lottoNumbers);
}
);
}

private void printPurchasedLotto(Lottos lottos) {
outputView.printPurchasedLotto(lottos);
}
}
33 changes: 33 additions & 0 deletions src/main/java/lotto/domain/lotto/LottoService.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package lotto.domain.lotto;

import java.util.ArrayList;
import java.util.List;
import lotto.domain.lotto.entity.Lotto;
import lotto.domain.lotto.entity.Lottos;
import lotto.domain.lotto.generator.LottoGenerator;
import lotto.domain.lotto.money.Cash;
import lotto.domain.lotto.money.Money;

public class LottoService {

public static final int LOTTO_PRICE = 1000;
private final LottoGenerator lottoGenerator;

public LottoService(LottoGenerator lottoGenerator) {
this.lottoGenerator = lottoGenerator;
}

public Lottos purchaseLottos(Money money) {
List<Lotto> lottos = new ArrayList<>();
Cash cash = money.toCash();
while (cash.canPurchase(LOTTO_PRICE)) {
lottos.add(generateLotto());
cash.spend(LOTTO_PRICE);
}
return new Lottos(lottos);
}

public Lotto generateLotto() {
return lottoGenerator.generate();
}
}
62 changes: 62 additions & 0 deletions src/main/java/lotto/domain/lotto/entity/Lotto.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package lotto.domain.lotto.entity;

import java.util.List;
import lotto.exception.LottoException;

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

Choose a reason for hiding this comment

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

stream ~ toList를 메서드로 분리하면 조금 더 깔끔해질 것 같아요!!

public Lotto(List<Integer> numbers) {
validate(numbers);
this.numbers = numbers.stream()
.map(LottoNumber::new)
.sorted()
.toList();
}

private void validate(List<Integer> numbers) {
validateSize(numbers);
validateDuplication(numbers);
}

private void validateDuplication(List<Integer> numbers) {
int distinctSize = (int) numbers.stream()
.distinct()
.count();
if (distinctSize != numbers.size()) {
throw LottoException.LOTTO_SIZE_EXCEPTION.makeException();
}
}

private static void validateSize(List<Integer> numbers) {
if (numbers.size() != 6) {
throw new IllegalArgumentException();
}
}

// TODO: 추가 기능 구현

Choose a reason for hiding this comment

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

getXXX 메서드 네이밍은
추후에 만약 getter 을 걷어내는 리팩터링을 수행한다면, 여러 메서드가 뒤섞여 추적하기 어려울 수도 있겠다는 생각이 들더라구요!
하지만 이를 대체할 수 있는 calculateXXX 는 메서드 네이밍이 너무 길어지는 것 같기도해서, 혁수님 생각이 궁금해요!

Copy link
Author

Choose a reason for hiding this comment

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

이 부분에 대해 이전에도 피드백을 받은 적이 있는데요.
아직 저만의 기준이 잡히진 않았고 저 역시 고민중인 주제입니다.

getter를 사용하는 부분은 내부 필드 변수를 가져오는 상황에 가장 맞는 네이밍인 것 같기도 해요.
다만 ~~한 정보를 가져온다. 라고 할 때 get을 제외하고 그 뉘앙스를 잘 살려줄 수 있는 다른 네이밍을 찾지는 못했기에 일단 임시방편으로 사용하는 중입니다.

상황에 따라 create~~와 같은 네이밍을 사용하거나, 조금 더 동작에 집중해 matchingNumbers와 같은 네이밍을 사용할 수도 있겠네요.

추가적으로 상황을 잘 설명하기 위함이라면 너~무 길지 않은 메서드 명은 오히려 좋다고 생각하는 편입니다!

Copy link

Choose a reason for hiding this comment

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

평균적으로 10~16자 정도의 변수 및 메소드 명을 사용하면 디버깅하기 쉽다고 알고 있습니다.

구글 코드 베이스도 변수명도 평균 10자 정도로 기억해요.

이런 경향은 언어에 따라 달라지기도 해서 Go언어 같은 경우는 this를 아예 클래스명의 첫글자로 축약해버립니다.

Person이면 p.name 이런식으로요.

Java는 간결한 것보다 명시적인 변수, 메소드명을 선호한다고 알고 있어서 calculateXXX 정도는 조금 길더라도 명시적이고 이해하기 어렵지 않은 이름이라고 생각됩니다.

말씀해주신대로 getter와 헷갈릴 여지도 줄일 수 있을 것 같아요.

너무 길다면 calcXXX 정도로 줄일 수도 있을 것 같습니다.

min, max, temp 등의 관례적으로 줄여서 쓰는 이름은 팀 간의 소통만 잘 하고 사용하면 문제가 없다고 봤는데, calc도 여기 해당되지 않나 싶어요.

지금은 개인 코드라 편한대로 사용해도 좋을 것 같네요.

public int getSameNumberCount(Lotto lotto) {
return (int) this.numbers.stream()
.filter(lotto.numbers::contains)
.count();
}

public boolean hasNumber(LottoNumber number) {
return this.numbers.contains(number);
}

public List<Integer> getNumbers() {
return numbers.stream()
.map(LottoNumber::getNumber)
.toList();
}


@Override
public String toString() {
return numbers.stream()
.map(LottoNumber::getNumber)
.toList()
.toString();
}
}
28 changes: 28 additions & 0 deletions src/main/java/lotto/domain/lotto/entity/LottoAnswer.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package lotto.domain.lotto.entity;

import lotto.exception.LottoException;

public class LottoAnswer {
private final Lotto lotto;
private final LottoNumber bonusNumber;

public LottoAnswer(Lotto lotto, int bonusNumber) {
this.lotto = lotto;
this.bonusNumber = new LottoNumber(bonusNumber);
validate();
}

private void validate() {
if (this.lotto.hasNumber(this.bonusNumber)) {
throw LottoException.LOTTO_DUPLICATED_BONUS_NUMBER.makeException();
}
}

public int getSameNumberCount(Lotto lotto) {
return lotto.getSameNumberCount(this.lotto);
}

public boolean matchesBonusNumber(Lotto lotto) {
return lotto.hasNumber(this.bonusNumber);
}
}
48 changes: 48 additions & 0 deletions src/main/java/lotto/domain/lotto/entity/LottoNumber.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package lotto.domain.lotto.entity;

import java.util.Objects;
import lotto.exception.LottoException;

public class LottoNumber implements Comparable<LottoNumber> {

public static final int LOTTO_RANGE_MAX = 45;
public static final int LOTTO_RANGE_MIN = 1;
Copy link

Choose a reason for hiding this comment

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

MIN이 prefix로 붙는 것이 좀 더 관례에 맞고 명시적이라는 느낌이 드는데 큰 문제는 없어 보입니다.

Copy link
Author

Choose a reason for hiding this comment

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

아 그런가요? 공통된 PREFIX를 사용해 값을 찾는데 더 도움이 되었으면 하는 생각으로 지은 네이밍이긴 합니다..!

private final int number;

public LottoNumber(int number) {
validateNumberRange(number);
this.number = number;
}

private void validateNumberRange(int number) {
if (number < LOTTO_RANGE_MIN || number > LOTTO_RANGE_MAX) {
throw LottoException.LOTTO_NUMBER_OUT_RANGE.makeException();
}
}

@Override
public int compareTo(LottoNumber o) {
return this.number - o.number;
}

public int getNumber() {
return number;
}

@Override
public boolean equals(Object o) {
if (this == o) {
return true;
}
if (o == null || getClass() != o.getClass()) {
return false;
}
LottoNumber that = (LottoNumber) o;
return number == that.number;
}

@Override
public int hashCode() {
return Objects.hash(number);
}
}
71 changes: 71 additions & 0 deletions src/main/java/lotto/domain/lotto/entity/LottoResult.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
package lotto.domain.lotto.entity;

import java.math.BigDecimal;
import java.util.Arrays;
import java.util.function.BiPredicate;

public enum LottoResult {
LOSE(new BigDecimal(0), 0, MatchBonusNumber.IGNORE),
FIFTH(new BigDecimal(5_000), 3, MatchBonusNumber.IGNORE),
FOURTH(new BigDecimal(50_000), 4, MatchBonusNumber.IGNORE),
THIRD(new BigDecimal(1_500_000), 5, MatchBonusNumber.NOT_MATCH),
SECOND(new BigDecimal(30_000_000), 5, MatchBonusNumber.MATCH),
FIRST(new BigDecimal(2_000_000_000), 6, MatchBonusNumber.IGNORE),
Comment on lines +8 to +13
Copy link

Choose a reason for hiding this comment

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

@zangsu
수식 계산의 안정성과 BigDecimal가 제공하는 기능을 위해서는 사용하는 게 맞다고 보는데, new로 생성하는 부분을 개선할 수 없을까요?

특성상 하나의 값들이 부여되어야 한다는 것을 머리로는 알지만, 여전히 고민 중인 부분이기에 코멘트 달아봤습니다. 🥲

Copy link
Author

Choose a reason for hiding this comment

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

어떤 점 때문에 new로 인스턴스를 생성하는 부분을 개선하시고 싶으신건가요?
고민하고 계신 부분을 조금 더 설명해 주시면 저도 좋은 고민거리가 될 수 있을 것 같습니다!

Copy link

Choose a reason for hiding this comment

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

@zangsu
예를 들면 테스트런타임 시 를 말씀드릴 수 있을 것 같아요.

제가 코드를 작성하며 하나의 인스턴스를 강조하는 이유는 불변성의 이유도 있지만 new를 통한 인스턴스 생성 비용을 줄이고자 하는 의도도 있습니다.

현재까지는 수식을 사용하는 데 BigDecimal를 사용하는 것이 최적이라는 생각을 하고 있는데, 위의 코드에서의 생성 비용과 테스트에서 도메인 객체를 단독으로 생성해야 하는 경우의 비용이 반복되면 결과적으로 느린 테스트와 실행시간이라는 이슈가 발생하지 않을까 하는 고민을 하고 있어요.

물론 그렇게 크지 않은 비용일지도 모르지만, 줄일 수 있다면 최적화에 도움이 되지 않을까 하는 마음에 코멘드 남겼습니다. 😄

아래는 참고를 위해 첨부한 구입한 로또 테스트에 사용되는 provider로직이에요.

    // List<LottoDto> 에 담아서 전달하기 위함.
    public static Stream<Arguments> provideForGenerateTest() {
        return Stream.of(
                Arguments.of(
                        List.of(
                                new LottoDto(List.of("1", "2", "3", "4", "5", "6"))
                        ), 1
                ),
                Arguments.of(
                        List.of(
                                new LottoDto(List.of("1", "2", "3", "4", "5", "6")),
                                new LottoDto(List.of("1", "2", "3", "4", "5", "10")),
                                new LottoDto(List.of("1", "2", "3", "6", "7", "15")),
                                new LottoDto(List.of("45", "44", "43", "42", "41", "40"))
                        ), 4
                ),
                Arguments.of(
                        List.of(
                                new LottoDto(List.of("1", "2", "3", "4", "5", "6")),
                                new LottoDto(List.of("1", "2", "3", "4", "5", "10")),
                                new LottoDto(List.of("1", "2", "3", "6", "7", "15")),
                                new LottoDto(List.of("45", "44", "43", "42", "41", "40")),
                                new LottoDto(List.of("1", "2", "3", "4", "5", "6")),
                                new LottoDto(List.of("1", "2", "3", "4", "5", "10")),
                                new LottoDto(List.of("1", "2", "3", "6", "7", "15")),
                                new LottoDto(List.of("45", "44", "43", "42", "41", "40"))
                        ), 8
                )
        );
    }

Copy link
Author

Choose a reason for hiding this comment

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

아하, 말씀해 주신 부분이 BigDecimal 자체를 static 상수로 가지고 있게 된다면 여러 군데에서 이미 생성되어 있는 상수를 가져와 사용할 수 있기에 조금 더 성능을 높일 수 있다고 이해를 하면 괜찮을까요?

여러 불변 클래스들이 자주 사용될 가능성이 높은 값들을 상수로 정의해 두고 이를 가져오기 위한 정적 팩토리 메서드를 제공하는 것과 느낌이 비슷할 것 같네요!
정말 좋습니다!!!

Copy link

@Hugh-KR Hugh-KR Nov 29, 2023

Choose a reason for hiding this comment

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

@zangsu
조금 다릅니다. 단독으로 생성되어야 하는 로직의 경우 static으로 가지고 있을 수 없어요.

정적으로 선언된 경우 새로운 매개변수가 들어오면 새로운 값으로 초기화되어 버리기 때문인데, 여기서 오는 딜레마로 고민이 깊어졌습니다. 😅

현시점에서서 엄청 중요한 내용은 아니고, 앞으로 알아가면 되는 부분이기에 너무 깊게 고민하지는 않으셨으면 해요.

;

private enum MatchBonusNumber {
MATCH(LottoAnswer::matchesBonusNumber),
NOT_MATCH(((lottoAnswer, lotto) -> !lottoAnswer.matchesBonusNumber(lotto))),
IGNORE((lottoAnswer, lotto) -> true),
;

private final BiPredicate<LottoAnswer, Lotto> matchBonusNumber;

MatchBonusNumber(BiPredicate<LottoAnswer, Lotto> matchBonusNumber) {
this.matchBonusNumber = matchBonusNumber;
}

public boolean checkBonusNumber(LottoAnswer lottoAnswer, Lotto lotto) {
return this.matchBonusNumber.test(lottoAnswer, lotto);
}
}

private final BigDecimal prize;
private final int sameCount;
private final MatchBonusNumber matchBonusNumber;

LottoResult(BigDecimal prize, int sameCount, MatchBonusNumber matchBonusNumber) {
this.prize = prize;
this.sameCount = sameCount;
this.matchBonusNumber = matchBonusNumber;
}

public static LottoResult getResult(LottoAnswer lottoAnswer, Lotto lotto) {
return Arrays.stream(LottoResult.values())
.filter(result -> result.matchesSameNumberCount(lottoAnswer, lotto))
.filter(result -> result.checkBonusNumber(lottoAnswer, lotto))
.findFirst()
.orElse(LOSE);
}
Comment on lines +43 to +49
Copy link

Choose a reason for hiding this comment

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

.orElse()을 사용하셨네요. 저는 이제 학습한 부분을 적용하신 모습이 정말 멋집니다. 🥲


private boolean matchesSameNumberCount(LottoAnswer lottoAnswer, Lotto lotto) {
int sameNumberCount = lottoAnswer.getSameNumberCount(lotto);
return this.sameCount == sameNumberCount;
}

private boolean checkBonusNumber(LottoAnswer lottoAnswer, Lotto lotto) {
return this.matchBonusNumber.checkBonusNumber(lottoAnswer, lotto);
}

public BigDecimal getPrize() {
return prize;
}

Choose a reason for hiding this comment

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

이렇게 하면, 플레이어 최종 상금을 계산할 때 getter를 안쓸 수 있겠군요! 👍

public BigDecimal getTotalPrize(int count) {
return this.prize.multiply(new BigDecimal(count));
}

public int getSameCount() {
return sameCount;
}
}
Loading