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

[로또] 어정윤 제출합니다. #2

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

[로또] 어정윤 제출합니다. #2

wants to merge 51 commits into from

Conversation

jeongyuneo
Copy link

Lotto에 책임이 많아서 어떻게 분리해야할지 고민을 많이 했는데, 주어진 테스트코드를 통과시키기 위해 LottoNumber 클래스를 생성하지 않고 그대로 진행했습니다.

고민했던 점은 로또번호를 생성할 때 구입수량에 따라 랜덤으로 생성되는 'IssuedLotto'와 당첨번호로 발행되는 'WinningLotto'를 Lotto 객체로 생성해서 네이밍만 달리해서 사용하고 있는데, 두 객체를 따로 관리해야할지였습니다.
두 객체의 역할이 같아서 Lotto 클래스 하나로 관리했는데, 분리하는게 더 좋을까요? 여러분의 의견이 듣고 싶습니다!

테스트코드 작성을 신경써서 최대한 작성해보려고 노력했는데, 랜덤으로 생성되는 값들이 포함되다보니 많이 작성하지 못했습니다.
테스트코드에 대한 조언과 충고, 혹은 잘못된 부분 지적 겸허히 받아들이겠습니다ㅎㅎ

Copy link
Member

@seong-wooo seong-wooo left a comment

Choose a reason for hiding this comment

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

고생하셨어요!
로또 미션 로직이 많이 어렵네요:(

src/main/java/lotto/domain/PurchasingMoney.java Outdated Show resolved Hide resolved
src/main/java/lotto/domain/Lotto.java Outdated Show resolved Hide resolved
Comment on lines 12 to 17
System.out.printf("\n%d개를 구매했습니다.%n", issuedLotto.getQuantity());
issuedLotto.getLottos()
.stream()
.map(Lotto::getNumbers)
.map(String::valueOf)
.forEach(System.out::println);
Copy link
Member

Choose a reason for hiding this comment

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

오 이렇게 간단하게 출력할 수 있군요

src/main/java/lotto/domain/Lottos.java Outdated Show resolved Hide resolved
src/main/java/lotto/domain/Lotto.java Outdated Show resolved Hide resolved
src/main/java/lotto/domain/Lotto.java Outdated Show resolved Hide resolved
Copy link

@ray-yhc ray-yhc left a comment

Choose a reason for hiding this comment

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

from과 create를 사용함으로써 객체 생성이 더 깔끔해진 것 같아요.
지난번 야구도 그렇고, 코드를 읽기 편하게 작성하시는 것 같다고 느꼈습니다.
수고 많으셨습니다~

@@ -0,0 +1,6 @@
package lotto.service;

public interface Manager {
Copy link

Choose a reason for hiding this comment

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

이건 질문인데요, interface와 구현체를 구분하신 것은 추후에 확장될 가능성이 있다고 생각하셨기 때문인가요?
어떤식으로 확장하는 걸 고려하셨나요?

Copy link
Author

Choose a reason for hiding this comment

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

제가 Manager로 추상화한 이유는

  1. 코드 구현부 캡슐화
    • interface로 추상화함으로써 '실행하다'라는 객체의 책임만 외부로 노출하고, 실제 실행을 수행하기 위한 코드 구현은 캡슐화할 수 있습니다.
  2. 교체 용이성
    • 해당 애플리케이션에서 로또 게임이 아닌 야구 게임을 실행하도록 교체한다고 했을 때, 타입을 변경하지 않고 야구 게임으로 쉽게 교체가 가능합니다.
      Manager manager = new NumberBaseballGameManager();    // 교체 전: Manager manager = new LottoManager();
      manager.run();

때문입니다!

Copy link

Choose a reason for hiding this comment

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

오옹..!! 저도 다음 과제부터는 참고해서 코드 작성해야겠습니다

src/main/java/lotto/service/LottoManager.java Outdated Show resolved Hide resolved
src/main/java/lotto/domain/PurchasingMoney.java Outdated Show resolved Hide resolved
Lotto winningLotto = Lotto.from(WinningLottoGenerator.from(Input.inputWinningLotto()));
BonusNumber bonusNumber = BonusNumber.from(winningLotto, Input.inputBonusNumber());

compareLotto(purchasingMoney, issuedLotto, winningLotto, bonusNumber);
Copy link

Choose a reason for hiding this comment

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

purchasingMoney.getQuantity()
issuedLotto.getQuantity()
issuedLotto.getLottos().size()

에서 모두 동일한 값을 꺼낼 수 있습니다!

Copy link
Author

Choose a reason for hiding this comment

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

Lottos에서 수량을 구하는 것은 불필요할 것 같아서 삭제했습니다!
출력을 위해 어쩔 수 없이 getLottos()를 사용해서 값을 꺼내와야 하는데 이때 size()purchasingMoney.getQuantity()와 같은 값을 가져오는 것을 막을 방법은 없는 것 같습니다ㅜㅜ

this.number = number;
}

public static BonusNumber from(Lotto lotto, String input) {
Copy link

Choose a reason for hiding this comment

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

BonusNumber의 생성자에서 lotto를 받아오는 게 어색합니다!
만약 winningLotto 보다 보너스 번호를 먼저 입력하도록 규칙이 변경된다면, 수정할 코드가 많아질 것 같아요.

Copy link
Author

Choose a reason for hiding this comment

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

오.. 거기까진 생각 못했습니다. 이런게 확장 가능한 설계인가요...!

제가 생각했을 때 보너스 번호의 의미가 당첨 번호를 다 뽑은 후 말 그대로 '보너스'로 뽑는 번호라고 생각해서, 앞서 뽑은 로또 번호들과 입력으로부터 생성되는게 어색하다고 생각하지 않았습니다.
먼저 뽑은 로또 번호가 없다면 보너스 번호도 없으니까요!

윤호님 피드백을 보고 당첨 번호와 보너스 번호를 저장하는 WinningLotto라는 클래스를 선언해보았는데, 이 방식으로 구현을 하다보니 BonusNumber는 검증로직조차 없는 오로지 값만 담고 있는 클래스가 되었습니다. 저는 BonusNumber가 의미있는 값이라고 생각해서 제가 의도한 바와 다른 것 같아 해당 부분은 수정하지 않았습니다.

혹시 또 의견주시면 즐겁게 의견에 대해 고민해보겠습니당ㅎㅎ

src/main/java/lotto/domain/result/WinningDetail.java Outdated Show resolved Hide resolved
src/main/java/lotto/domain/Lotto.java Outdated Show resolved Hide resolved
Copy link
Member

@seong-wooo seong-wooo 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 +10 to +11
private Input() {
}
Copy link
Member

Choose a reason for hiding this comment

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

이런 클래스 같은 경우 아예 생성자를 사용할 수 없게 생성자 내부에서 예외를 터뜨리는건 어떨까요?
private 으로 선언하면 외부에서 생성자를 사용할 수 없지만, 리플랙션 같은 기술로 사용할 수는 있을 테니까요.
예외를 터뜨림으로써 사용되지 않는 생성자임을 명시하는 것도 좋을 것 같아요.

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.

3 participants