[로또] 레몽(이규명) 미션 제출합니다. #2

lgm1007 · 2024-05-19T14:05:42Z

기능 요구사항

로또 구매

구입 금액을 입력하면 구입 금액에 해당하는 만큼 로또를 발급한다.
로또 1장의 가격은 1000원이다.

지난 주 당첨 번호 비교

지난 주 당첨 번호와 보너스 볼 번호를 입력받는다.
발급된 로또와 비교하여 당첨 통계를 생성한다.
- 3개 일치 (5000원)
- 4개 일치 (50000원)
- 5개 일치 (1500000원)
- 5개 일치, 보너스 볼 일치(30000000원)
- 6개 일치 (2000000000원)
총 수익률을 계산해서 출력해준다.
- 수익률은 구매한 금액 / 총 당첨 금액 계산값을 소수점 두 자리 수까지 나타낸 수이다.

수동 로또 구매

로또 구입 금액 입력 후 수동 구매 수량을 입력받는다.
수동 구매 수량을 입력받은 후 수동으로 구매할 로또 번호를 구매 수량만큼 입력받는다.
자동 구매권 수량은 총 구매한 개수 - 수동 구매 수량이다.

lgm1007 · 2024-05-19T14:07:38Z

미션 제출해요~!
당첨 종류에 대해서 Enum을 사용해 표현했는데, Enum을 효과적으로 사용하진 못한 느낌이 들어 살짝 아쉬운 점이 남은 미션이었네요🤣

haero77

레몽 안녕하세요~! 리뷰어 해로입니다!
레몽 리뷰는 처음이라 가볍게(?) 남겨봤습니다~!

haero77 · 2024-05-21T12:00:21Z

src/main/java/Application.java

+	private static LottoNumberCollectionList receivePhraseLotto(final AmountRequest amountRequest, final ManualNumberRequest manualNumberRequest) {
+		PhraseLottoExecutor phraseLottoExecutor = new PhraseLottoExecutor(amountRequest, manualNumberRequest);
+		phraseLottoExecutor.phraseLotto();
+		return phraseLottoExecutor.pickLottoNumber();
+	}
+
+	private static WinningTypeCollection retrieveWinning(final LottoNumberCollectionList lottoNumberCollectionList) {
+		WinningNumberRequest winningNumberRequest = InputView.inputWinningNumber();
+		BonusNumberRequest bonusNumberRequest = InputView.inputBonusNumber();
+		WinningConfirmExecutor winningConfirmExecutor = new WinningConfirmExecutor(lottoNumberCollectionList, winningNumberRequest, bonusNumberRequest);
+		return winningConfirmExecutor.confirmWinningType();
+	}


Application의 역할을 무엇으로 보셨는지 궁금합니다 :)
receivePhraseLotto, retrieveWinning 등의 로직을 Application에 두신 이유가 있을까요?

안녕하세요 해로님! 성심성의껏 리뷰 남겨주셔서 감사해요😊
Application은 의미 그대로 애플리케이션을 구동하는 역할을 수행해야 한다고 생각해요.
그래서 위 receivePhraseLotto, retrieveWinning 메서드도 보시면 Executor을 실행한 반환값을 전달해주는 메서드로 구현되었는데, 이제 보니 각 수행 동작별로 Executor를 나눠놓고 이들을 모두 Application에서 처리하려다보니 해로님의 질문이 나오게 된 것 같아요!

(receive, retrieve 의미는 비슷한데 혼용해서 메서드 네이밍을 쓴 것도 지금보니 눈에 걸리는 포인트네요...😖)

Application은 의미 그대로 애플리케이션을 구동하는 역할을 수행해야 한다고 생각해요.

WinningNumberRequest winningNumberRequest = InputView.inputWinningNumber(); BonusNumberRequest bonusNumberRequest = InputView.inputBonusNumber()

애플리케이션 '구동'을 어디까지로 볼지에 대한 정의를 먼저 내려보면 좋을 것 같아요.
지금은 구동을 넘어 사용자 입력까지 모두 처리하고 있어서, 구동의 역할을 넘어선 걸로 보이네요 :D

haero77 · 2024-05-21T12:02:24Z

src/main/java/controller/AmountRequest.java

+
+import validator.AmountValidator;
+
+public class AmountRequest {


Amount의 뭘 하길 위한 Request인지 예측하기 어려운 네이밍 같아요.

그냥 금액을 입력 받기 위한 값 객체인줄 알았는데 실제로는 로또 금액을 관리하고 있군요 :)

Amount는 단어 의미 그대로 로또의 수량에 대한 의미로 사용했고, AmountRequest는 로또의 수량을 전달해주는 Request라는 의미로 사용했어요.

로또의 수량은 구매 금액 / 로또 금액으로 구해지다보니 로또의 금액을 해당 AmountRequest에서 로또의 금액도 관리하게 되었어요!

Amount는 단어 의미 그대로 로또의 수량에 대한 의미로 사용했고,

로또의 수량에 대한 의미로 사용하려면, LottoCount 등으로 네이밍을 더 명확하게 좁혀야 혼란이 줄어들 것 같아요.
Amount 그 자체는 단순 금액에 대한 값 객체 외에는 다른 의미를 떠올리기 힘든 편입니다 :)

src/main/java/controller/AmountRequest.java

haero77 · 2024-05-21T12:10:23Z

src/main/java/controller/AmountRequest.java

+	public int fetchPhraseLottoCount() {
+		return (int) this.lottoAmount / LOTTO_PRICE;
+	}


금액별로 로또를 몇 장 구매할 수 있는지 계산해야하는 책임을 Request 객체가 가지는 것은 조금 어색하다고 느꼈어요. XXXRequest 라서 단순 DTO 같은데 꽤 중요한 로직이 여기 있어도 될까라는 생각이 들었습니다 ㅎㅎ

아하 그렇게 받아드릴 수도 있을 것 같아요!

저는 AmountRequest는 로또 수량에 대한 책임을 지닌 객체로 판단해서 해당 Request에서 로또 수량에 대한 내용을 은연중에 부여한 것도 있는 것 같아요ㅎㅎ 그러다보니 자연스럽게 로또 수량을 계산하는 로직도 위임했던 것 같은데, Request를 실제 사용자에게 입력받는 값에 대해서로만 구현했다면 더 괜찮았을 것 같다고 생각이 드네요!
그렇게 된다면 로또 수량이 아닌 로또를 구매한 금액에 대한 Request로 구현할 수 있게 되겠어요

haero77 · 2024-05-21T12:47:52Z

src/main/java/validator/InputValidator.java

+	private static final String WINNING_NUMBER_NOT_NUMBER_MESSAGE = "[ERROR] 당첨 번호는 1부터 45까지의 숫자로 이루어져야 합니다.";
+	private static final String BONUS_NUMBER_NOT_NUMBER_MESSAGE = "[ERROR] 보너스 볼 번호는 1부터 45까지의 숫자로 이루어져야 합니다.";
+	private static final String NUMBER_INPUT_SPLIT_REGEX = ",";
+	private static final int LOTTO_NUMBER_COUNT = 6;


이 6이라는 숫자는 다른 클래스에서도 본 것 같습니다!
여러 클래스에 걸쳐 같은 상수를 정의하신 이유가 궁금해요 :)

haero77 · 2024-05-21T12:51:36Z

src/main/java/view/InputView.java

+	private static final String INPUT_AMOUNT_MESSAGE = "구입금액을 입력해 주세요.";
+	private static final String INPUT_MANUAL_COUNT_MESSAGE = "수동으로 구매할 로또 수를 입력해 주세요.";
+	private static final String INPUT_MANUAL_NUMBER_MESSAGE = "수동으로 구매할 번호를 입력해 주세요.";
+	private static final String INPUT_WINNING_NUMBER_MESSAGE = "지난 주 당첨 번호를 입력해 주세요.";
+	private static final String INPUT_BONUS_NUMBER_MESSAGE = "보너스 볼을 입력해 주세요.";
+
+	public static AmountRequest inputAmount() {
+		System.out.println(INPUT_AMOUNT_MESSAGE);


한 번만 사용되는 상수이고, 중요한 비즈니스 정책을 담은 것도 아니라서 상수로 빼야할지 고민되네요 :)
레몽의 상수를 사용하는 기준이 궁금합니다!

저는 하드 코딩되는 문자열이나 정수 값에 대해서는 상수로 만들어 사용하고 있는데, 위와 같이 뷰 출력 메시지나 에러 메시지에 대한 건 눈으로 바로 보이는 값이기 때문에 중복으로 사용되는 값이 아니라면 굳이 상수로 만들지 않아도 되겠어요!

하지만 이렇게 눈으로 보이는 값이 아닌 하드 코딩된 값은 상수로 관리하는 편이 추후 추적하기 용이하다고 판단되어 상수로 관리하는 편이 저는 좋더라구요~!

haero77 · 2024-05-21T12:53:56Z

src/main/java/view/InputView.java

+	}
+
+	public static ManualNumberRequest inputManual(final int lottoAmount) {
+		System.out.println(INPUT_MANUAL_COUNT_MESSAGE);


여기에는 오히려 하드 코딩하는게 가독성이 더 좋아보입니다!
(메시지를 확인하기 위해 굳이 위아래로 스크롤 하지 않아도 되고요!)

haero77 · 2024-05-21T12:57:03Z

src/main/java/view/InputView.java

+	public static WinningNumberRequest inputWinningNumber() {
+		System.out.println(INPUT_WINNING_NUMBER_MESSAGE);
+		try {
+			final String winningNumberInput = Console.readLine();
+			InputValidator.validateWinningNumberInput(winningNumberInput);
+			return WinningNumberRequest.from(winningNumberInput);
+		} catch (IllegalArgumentException e) {
+			System.out.println(e.getMessage());
+			return inputWinningNumber();
+		}
+	}
+
+	public static BonusNumberRequest inputBonusNumber() {
+		System.out.println(INPUT_BONUS_NUMBER_MESSAGE);
+		try {
+			final String bonusNumberInput = Console.readLine();
+			InputValidator.validateBonusNumberInput(bonusNumberInput);
+			return BonusNumberRequest.from(bonusNumberInput);
+		} catch (IllegalArgumentException e) {
+			System.out.println(e.getMessage());
+			return inputBonusNumber();
+		}
+	}


입력을 받고, 유효성을 검증하고, 예외를 처리 & 잡고, 예외 메시지를 출력하는 로직이 클래스 내에서 4회 이루어지고 있어요.
이런 중복되는 구조의 경우 generic하게 만들어서 조금 더 간결하게 표현해도 좋다고 생각합니다 :)

src/main/java/vo/LottoNumberCollection.java

lgm1007

해로님! 우선 성심성의껏 이렇게 리뷰 남겨주셔서 감사해요! 덕분에 제가 어떤 포인트를 놓치고 있는지 배울 수 있었어요! 우선 남겨주신 리뷰에 대해 절반 정도 답변 남깁니다😄

lgm1007 · 2024-05-22T14:33:52Z

src/main/java/Application.java

+	private static LottoNumberCollectionList receivePhraseLotto(final AmountRequest amountRequest, final ManualNumberRequest manualNumberRequest) {
+		PhraseLottoExecutor phraseLottoExecutor = new PhraseLottoExecutor(amountRequest, manualNumberRequest);
+		phraseLottoExecutor.phraseLotto();
+		return phraseLottoExecutor.pickLottoNumber();
+	}
+
+	private static WinningTypeCollection retrieveWinning(final LottoNumberCollectionList lottoNumberCollectionList) {
+		WinningNumberRequest winningNumberRequest = InputView.inputWinningNumber();
+		BonusNumberRequest bonusNumberRequest = InputView.inputBonusNumber();
+		WinningConfirmExecutor winningConfirmExecutor = new WinningConfirmExecutor(lottoNumberCollectionList, winningNumberRequest, bonusNumberRequest);
+		return winningConfirmExecutor.confirmWinningType();
+	}


안녕하세요 해로님! 성심성의껏 리뷰 남겨주셔서 감사해요😊
Application은 의미 그대로 애플리케이션을 구동하는 역할을 수행해야 한다고 생각해요.
그래서 위 receivePhraseLotto, retrieveWinning 메서드도 보시면 Executor을 실행한 반환값을 전달해주는 메서드로 구현되었는데, 이제 보니 각 수행 동작별로 Executor를 나눠놓고 이들을 모두 Application에서 처리하려다보니 해로님의 질문이 나오게 된 것 같아요!

(receive, retrieve 의미는 비슷한데 혼용해서 메서드 네이밍을 쓴 것도 지금보니 눈에 걸리는 포인트네요...😖)

lgm1007 · 2024-05-22T14:36:09Z

src/main/java/controller/AmountRequest.java

+
+import validator.AmountValidator;
+
+public class AmountRequest {


Amount는 단어 의미 그대로 로또의 수량에 대한 의미로 사용했고, AmountRequest는 로또의 수량을 전달해주는 Request라는 의미로 사용했어요.

로또의 수량은 구매 금액 / 로또 금액으로 구해지다보니 로또의 금액을 해당 AmountRequest에서 로또의 금액도 관리하게 되었어요!

src/main/java/controller/AmountRequest.java

lgm1007 · 2024-05-22T14:46:24Z

src/main/java/controller/AmountRequest.java

+	public int fetchPhraseLottoCount() {
+		return (int) this.lottoAmount / LOTTO_PRICE;
+	}


아하 그렇게 받아드릴 수도 있을 것 같아요!

저는 AmountRequest는 로또 수량에 대한 책임을 지닌 객체로 판단해서 해당 Request에서 로또 수량에 대한 내용을 은연중에 부여한 것도 있는 것 같아요ㅎㅎ 그러다보니 자연스럽게 로또 수량을 계산하는 로직도 위임했던 것 같은데, Request를 실제 사용자에게 입력받는 값에 대해서로만 구현했다면 더 괜찮았을 것 같다고 생각이 드네요!
그렇게 된다면 로또 수량이 아닌 로또를 구매한 금액에 대한 Request로 구현할 수 있게 되겠어요

lgm1007 · 2024-05-22T15:04:15Z

src/main/java/strategy/FiveBonusMatchWinningStrategy.java

+public class FiveBonusMatchWinningStrategy implements WinningPriceStrategy {
+	@Override
+	public WinningType fetchWinningType() {
+		return WinningType.FIVE_BONUS_MATCH;
+	}


만들었을 당시에는 당첨 조건이나 각 당첨 등수에 대한 분기에 해당하는 로직을 전략으로 만들어서 구현해보자~ 라는 생각으로 만들었긴 했습니다.
현재 제출한 시점에서 바라본 해당 Strategy들은 일치한 번호 개수와 보너스 번호 일치 유무로 당첨 등수를 반환해주는 역할을 수행하고 있다고 볼 수 있을 것 같아요.
즉, 일치한 번호 개수와 보너스 번호 일치 유무 값으로 분기 처리해서 구했을 등수를 전략 패턴을 사용하여 조금 간단하게 구현하도록 도움을 주기 위한 전략 패턴이라고 보여요

lgm1007 · 2024-05-22T15:06:18Z

src/main/java/validator/AmountValidator.java

+	public static void validateAmountNegative(final int lottoAmount) {
+		if (lottoAmount < 0) {
+			throw new IllegalArgumentException(NEGATIVE_EXCEPTION_MESSAGE);
+		}
+	}


클래스 네이밍의 실책이네요..! 사용자 입력에 대한 Validation을 하고자 한다면 금액이라는 네이밍을 사용했어야 옳게 된 Validator라고 생각이 듭니다💡

lgm1007 · 2024-05-22T15:07:16Z

src/main/java/validator/AmountValidator.java

+public class AmountValidator {
+
+	private static final String NEGATIVE_EXCEPTION_MESSAGE = "[ERROR] 금액은 음수로 입력할 수 없습니다.";
+
+	public static void validateAmountNegative(final int lottoAmount) {


앗 private 생성자가 존재하지 않는 점에 대한 의미로 받아들였는데 맞을까요?

lgm1007 · 2024-05-22T15:11:54Z

src/main/java/view/InputView.java

+	private static final String INPUT_AMOUNT_MESSAGE = "구입금액을 입력해 주세요.";
+	private static final String INPUT_MANUAL_COUNT_MESSAGE = "수동으로 구매할 로또 수를 입력해 주세요.";
+	private static final String INPUT_MANUAL_NUMBER_MESSAGE = "수동으로 구매할 번호를 입력해 주세요.";
+	private static final String INPUT_WINNING_NUMBER_MESSAGE = "지난 주 당첨 번호를 입력해 주세요.";
+	private static final String INPUT_BONUS_NUMBER_MESSAGE = "보너스 볼을 입력해 주세요.";
+
+	public static AmountRequest inputAmount() {
+		System.out.println(INPUT_AMOUNT_MESSAGE);


저는 하드 코딩되는 문자열이나 정수 값에 대해서는 상수로 만들어 사용하고 있는데, 위와 같이 뷰 출력 메시지나 에러 메시지에 대한 건 눈으로 바로 보이는 값이기 때문에 중복으로 사용되는 값이 아니라면 굳이 상수로 만들지 않아도 되겠어요!

하지만 이렇게 눈으로 보이는 값이 아닌 하드 코딩된 값은 상수로 관리하는 편이 추후 추적하기 용이하다고 판단되어 상수로 관리하는 편이 저는 좋더라구요~!

src/main/java/vo/LottoNumberCollection.java

lgm1007 added 15 commits May 12, 2024 12:50

docs: 기능 요구사항 추가

cc2f768

feat: 구매 금액 입력, 구매 개수만큼 로또 번호 출력

1177834

modify: LottoNumberCollection 구성 필드 수정

27be596

feat: 지난 주 당첨 번호 입력받기

aa533b6

feat: 지난주 번호 맞춘 개수가 몇 개인지 통계 출력

64baade

feat: 수익률 계산 및 출력

0b778be

modify: 당첨 통계 출력메시지 출력

ef5951f

modify: 로또 당첨번호 입력받을 시 공백 제거

3183b40

feat: 수동 구매 입력

2deb60b

feat: 수동으로 구매한 번호 검증 시 확인하기

6d71e62

test: 입력 유효성 검사 테스트코드 작성

1d4105d

refactor: 유효성 검사 중 분기 로직 메서드 추출

7afd95c

test: 당첨 금액 통계 출력 테스트코드 작성

81b2527

test: 로또 구입 출력 메시지 테스트코드 작성

e029076

docs: 기능 요구사항 수동 로또 구매 내용 추가

68ebdcc

lgm1007 requested review from monsteralover and haero77 May 19, 2024 14:05

haero77 reviewed May 21, 2024

View reviewed changes

lgm1007 commented May 22, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[로또] 레몽(이규명) 미션 제출합니다. #2

[로또] 레몽(이규명) 미션 제출합니다. #2

lgm1007 commented May 19, 2024

lgm1007 commented May 19, 2024

haero77 left a comment

haero77 May 21, 2024

lgm1007 May 22, 2024

haero77 May 23, 2024 •

edited

Loading

haero77 May 21, 2024

lgm1007 May 22, 2024

haero77 May 23, 2024 •

edited

Loading

haero77 May 21, 2024 •

edited

Loading

lgm1007 May 22, 2024

haero77 May 21, 2024

haero77 May 21, 2024

lgm1007 May 22, 2024

haero77 May 21, 2024

haero77 May 21, 2024 •

edited

Loading

lgm1007 left a comment

lgm1007 May 22, 2024

lgm1007 May 22, 2024

lgm1007 May 22, 2024

lgm1007 May 22, 2024

lgm1007 May 22, 2024

lgm1007 May 22, 2024

lgm1007 May 22, 2024


		import validator.AmountValidator;

		public class AmountRequest {

[로또] 레몽(이규명) 미션 제출합니다. #2

Are you sure you want to change the base?

[로또] 레몽(이규명) 미션 제출합니다. #2

Conversation

lgm1007 commented May 19, 2024

기능 요구사항

로또 구매

지난 주 당첨 번호 비교

수동 로또 구매

lgm1007 commented May 19, 2024

haero77 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

haero77 May 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

haero77 May 23, 2024 • edited Loading

Choose a reason for hiding this comment

haero77 May 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

haero77 May 21, 2024 • edited Loading

Choose a reason for hiding this comment

lgm1007 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

haero77 May 23, 2024 •

edited

Loading

haero77 May 23, 2024 •

edited

Loading

haero77 May 21, 2024 •

edited

Loading

haero77 May 21, 2024 •

edited

Loading