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

[로또 미션] 황현식 미션 제출합니다. #44

Open
wants to merge 7 commits into
base: choon0414
Choose a base branch
from

Conversation

Choon0414
Copy link

코드에 대해 고민을 많이 해보는 좋은 시간이였던 것 같습니다.

다 짜고 나서 리팩터링 하는건 참 힘들다는 걸 깨달았습니다..
앞으론 처음부터 많이 생각하면서 작성하겠습니다.

코드가 아직 지저분해서 따로 더 고민해볼 생각입니다.

리뷰 잘 부탁드립니다!

public class Constant {

public static final int MAX_NUM = 45;
public static final int MIN_NUM = 1;

Choose a reason for hiding this comment

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

상수만 따로 모아놓셨군요!


private void verifyRangeNumber(int number) {
if (number > Constant.MAX_NUM || number < Constant.MIN_NUM) {
throw new IllegalArgumentException("잘못된 범위의 숫자 입니다. 번호: " + number);

Choose a reason for hiding this comment

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

예외처리 좋습니당

Comment on lines 26 to 29
if (this == o)
return true;
if (o == null || getClass() != o.getClass())
return false;

Choose a reason for hiding this comment

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

코드블럭이 없어요...!

Copy link
Author

Choose a reason for hiding this comment

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

앗 수정하겠습니다

private final List<Integer> numbers;

public RandomNumberGenerator() {
numbers = IntStream.rangeClosed(Constant.MIN_NUM, Constant.MAX_NUM)

Choose a reason for hiding this comment

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

IntStream Good

Comment on lines +43 to +49
try {
Winnings winnings = Winnings.of(matchCount, bonusNumber);
this.winningsResult.merge(winnings, 1, Integer::sum);
prize += winnings.getWinningPrize();
} catch (NoSuchElementException ignore) {

}

Choose a reason for hiding this comment

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

어떤 역할을 하는 try-catch일까요?!

Copy link
Author

Choose a reason for hiding this comment

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

일치하는 수가 3개 미만인 것들은 무시하려고 쓴 부분인데
등수 외를 나타내는 상수를 쓰는게 더 좋은 방법 같네요!

Comment on lines 18 to 34
for (Lotto lotto : lottoList) {
List<Integer> lottoNumber = lotto.getLottoNumber().stream()
.map(LottoNumber::getNumber)
.toList();
System.out.println(lottoNumber);
}
}

public static void printWinningState(Map<Winnings, Integer> result, double rateOfResult) {
System.out.println("당첨 통계 \n--------");
for (Winnings winnings : Winnings.values()) {
System.out.println(String.format(getResultMessage(winnings),
winnings.getMatchCount(),
winnings.getWinningPrize(),
result.get(winnings)));
}
System.out.println(String.format("총 수익률은 %.2f입니다.", rateOfResult));

Choose a reason for hiding this comment

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

라인 포멧팅이 안되어 있는 것 같습니다!

Comment on lines +37 to +41
public static String getResultMessage(Winnings winnings) {
if (winnings == Winnings.SECOND_PLACE) {
return "%d개 일치, 보너스 볼 일치(%d원)- %d개";
}
return "%d개 일치 (%d원)- %d개";

Choose a reason for hiding this comment

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

따로 모듈화 해주셨군요 좋습니당

import domain.LottoNumber;
import domain.LottoStatus;

class LottoTest {

Choose a reason for hiding this comment

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

오.. 테스트 굿

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants