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

java-lotto step2 #3265

Open
wants to merge 11 commits into
base: prolkh
Choose a base branch
from
Open

java-lotto step2 #3265

wants to merge 11 commits into from

Conversation

prolkh
Copy link

@prolkh prolkh commented May 31, 2023

잘 부탁드립니다.
감사합니다. 😌

Copy link

@nokchax nokchax left a comment

Choose a reason for hiding this comment

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

안녕하세요, 경호님!
오랜만에 인사 드리는 것 같네요
로또 미션 잘 진행해 주셨습니다 👍
자잘한 피드백 대신 큰 피드백 위주로 남겼는데요,
확인 부탁드립니다. 🙇

Comment on lines +24 to +27
@Override
public String toString() {
return lottoBuyCount + "개를 구매했습니다.\n" + lottoTickets;
}
Copy link

Choose a reason for hiding this comment

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

도메인(model) 내부에 출력(view)과 관련된 로직이 들어가 있으면 안 돼요
model 과 view 를 분리해주세요~!

private final int lottoBuyCount;

LottoBuyCount(int buyPrice) {
this.lottoBuyCount = buyPrice / LOTTO_PRICE;
Copy link

Choose a reason for hiding this comment

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

buyPrice 가 음수이거나 0 일 경우는 어떤 일이 벌어지나요?

public class LottoNumberGenerator {

public List<Integer> generateLottoNumbers() {
List<Integer> lottoNumbers = new ArrayList<>();
Copy link

Choose a reason for hiding this comment

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

1부터 45 까지의 lottoNumber 도 LottoNumber 클래스로 래핑해주세요~!

}

public Map<Integer, Integer> calculateLottoWinStatistics(LottoWinNumber lottoWinNumber) {
Map<Integer, Integer> resultMap = new HashMap<>();
Copy link

Choose a reason for hiding this comment

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

변수명에 컬렉션 이름(list, map, set 등)을 사용하지 말아주세요, 객체지향적인 사고를 저해하는 요소입니다.

}

public Map<Integer, Integer> calculateLottoWinStatistics(LottoWinNumber lottoWinNumber) {
Map<Integer, Integer> resultMap = new HashMap<>();
Copy link

Choose a reason for hiding this comment

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

result 도 별도의 클래스로 추출해 보면 어떨까요?


import java.util.Map;

public class LottoWinStatistics {
Copy link

Choose a reason for hiding this comment

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

로또 결과는 일치하는 갯수와 당첨금으로 구성되어 있습니다.
이는 변하지 않는 값인데요 enum으로 구현해보면 어떨까요?

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