Skip to content

[2주차] 객체지향 코드 연습(chiseungan)#53

Open
chiseungan wants to merge 1 commit intoCOW-edu:mainfrom
chiseungan:main
Open

[2주차] 객체지향 코드 연습(chiseungan)#53
chiseungan wants to merge 1 commit intoCOW-edu:mainfrom
chiseungan:main

Conversation

@chiseungan
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown

@hughryu1125 hughryu1125 left a comment

Choose a reason for hiding this comment

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

수고하셨습니다!

기본적으로 model, view, controller간 분리가 깔끔하게 되어 있는 것 같아요!
지금 코드를 토대로 조금 리팩토링과 App config를 이용해서 DI 적용하면 될 것 같습니다!

Comment on lines +13 to +20
public class LottoController {
private final InputView inputView;
private final OutputView outputView;

public LottoController() {
this.inputView = new InputView();
this.outputView = new OutputView();
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

이 부분에 DI를 적용해주시면 됩니다!

private List<Lotto> generateLottos(int count) {
List<Lotto> lottos = new ArrayList<>();
for (int i = 0; i < count; i++) {
List<Integer> numbers = Randoms.pickUniqueNumbersInRange(1, 45, 6);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

매직넘버 처리해주세요!

그리고 지금은 로또의 넘버들을 controller 내부 method로 만들고 있는데 내부 로직은 model이 갖는게 어떨까요?

}

private void validate(List<Integer> numbers) {
if (numbers.size() != 6) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

여기도 매직넘버 처리해주세요!


private void validate(List<Integer> numbers) {
if (numbers.size() != 6) {
throw new IllegalArgumentException("[ERROR] 로또 번호는 6개여야 합니다.");
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Error message는 한 곳에서 관리하면 좋을 것 같아요!

Comment on lines +22 to +23
if (new HashSet<>(numbers).size() != 6) {
throw new IllegalArgumentException("[ERROR] 로또 번호는 중복될 수 없습니다.");
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

매직넘버와 에러 분리 처리해주세요!
이 두 개 관련해선 이제 리뷰 남기지 않을게요!

controller.run();
}
}
} No newline at end of file
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

EOL 이슈 수정해주세요!

다른 파일들도 모두 수정해주세요..!

https://share.google/2ij7TC91qN9vjFAzh


private int parsePurchaseAmount(String input) {
try {
int amount = Integer.parseInt(input);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

파싱은 누가 책임을 져야 할 것인가를 생각해보시면 좋을 것 같아요!

Comment on lines +38 to +47
private List<Integer> parseWinningNumbers(String input) {
try {
return Arrays.stream(input.split(","))
.map(String::trim)
.map(Integer::parseInt)
.collect(Collectors.toList());
} catch (NumberFormatException e) {
throw new IllegalArgumentException("[ERROR] 로또 번호는 숫자여야 합니다.");
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

여기도 파싱의 책임을 분리하는 방향도 생각해보시면 좋을 것 같아요!

지금은 view의 책임이 조금 많아보여요

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