Skip to content

[이예진] 로또 1,2단계 구현 #126

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

Open
wants to merge 5 commits into
base: yaevrai
Choose a base branch
from

Conversation

yaevrai
Copy link

@yaevrai yaevrai commented May 26, 2025

구현이 늦어져서 죄송합니다🙏

이번 미션을 통해 일급 컬렉션이란 개념에 대해 알게되었는데요, 언뜻 보면 쉬우면서 애매하게 잘 안와닿았던 개념인데,
잘 설명해 주셔서 너무 감사합니다.

회고

  • 배열 대신 컬렉션을 사용한다는 이번 요구사항을 통해 평소에 배열, 리스트, 컬렉션에 관해 명확한 이해를 하지 않고있다는 걸 깨달았습니다.
  • 일급 컬렉션, 원시값을 사용하지 않는다는 요구사항을 통해 강제성을 두고 구현을 하다보니 좀 더 응집도 높은 코드, 분리되고 명확한 코드에 대한 이해도가 높아졌습니다 🙌👍

public class LottoController {

public static void main(String[] args) {
LottoService service = new LottoService(new LottoInputView(), new LottoOutputView());
Copy link
Author

Choose a reason for hiding this comment

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

  1. 지난 미션에선 Main이란 이름의 클래스에서 Controller를 조작하도록 구현했었는데,
    Controller가 하는 일과 네이밍이 잘 안맞는 듯해서 이번엔 Controller내 메인 클래스를 두고 프로그램 조작만을 담당하도록 했습니다.!

  2. 지난 미션에선 하나의 View 객체를 만들어서 입/출력 관련 역할을 모았었는데요,
    스캐너를 사용하는 건 같지만 출력/입력은 다른 역할이라는 생각이 들어 입/출력 View 클래스를 나누어 보았습니다.

Choose a reason for hiding this comment

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

예진님이 생각하시는 Controller의 역할은 무엇인가요? 그렇다면 Service의 역할은 무엇인가요?

Copy link
Author

Choose a reason for hiding this comment

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

Controller : 외부 요청을 받고, 그 요청을 처리할 적절한 로직으로 연결해주는 역할
Service : 비즈니스 로직을 담당하는 역할

이번 미션에선 요청이라고는 할 수 없지만, 실행 및 흐름을 조작을 하는 역할을 하고, 나머지 기능은 서비스단에서 구현하도록 했습니ㄷ!


List<Lotto> lottos = purchaseLottos(purchaseMoney);

LottoTicket lottoTicket = purchaseMoney.calculateTicketCount();
Copy link
Author

Choose a reason for hiding this comment

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

LottoTicket은 로또를 몇개 구입했는 지에 대한 정보를 가진 객체입니다.
원시값은 사용하지 않는 다는 이번 요구사항이 있는데, 만약 이 메서드 내에서
int lottoTicketCount = purchaseMoney.calculateTicketCount().getCount();를 해버리면 요구사항에 어긋나게 되는 걸까요?

int lottoTicketCount으로 사용하게 되면 이 메서드 내에서만 사용하는 거니 중복에 대한 고려가 없어도 되고, 오히려 더 명시적이지 않은가? 라는 고민을 했었습니답!

Choose a reason for hiding this comment

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

LottoTicket이 정말 필요한 객체일까요?

요구사항에서는 모든 원시값을 포장하라고 했어요.

그렇다고 프로젝트에 사용되는 모든 원시값을 포장하면 어떻게 될까요?

Copy link
Author

Choose a reason for hiding this comment

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

그렇다고 프로젝트에 사용되는 모든 원시값을 포장하면 어떻게 될까요?

저도 진행하면서 정말 필요할까라는 생각을 했는데, 요구사항인 모든 이라는 말에 아닌 거 같으면서도 모든 원시값을 포장했네요😅
어제 미팅에서 처럼 요구사항이라 할지라도 아닌 것 같을 땐 의문을 가지는 자세를 가져야 할 것 같아요!

return value;
}

public LottoTicket calculateTicketCount() {
Copy link
Author

Choose a reason for hiding this comment

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

로또를 몇개 구매했는지 알고싶다면 얼마를 지불했는지 알아야하니까 Money객체 내에서 계산하도록 했는데요,
로직상으론 문제 없는 것 같은데 뭔가 Money라는 객체에서 로또를 몇개 구입했는지 아는게 명시적인가? 라는 고민이 들었습니다

Choose a reason for hiding this comment

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

로또의 구매라는 역할을 하는 클래스가 있으면 되지 않을까요?

Copy link
Author

@yaevrai yaevrai Jun 1, 2025

Choose a reason for hiding this comment

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

PurchaseLotto라는 클래스를 만들어서 역할을 나눠보았습니다 !
그런데 그러면 일급컬렉션이어야한다는 요구사항에 어긋나게 되는 걸까요?ㅠㅠ

matchResults.forEach((matchCount, count) ->
statistics.put(String.valueOf(matchCount), (long) count));

long totalPrize = matchResults.entrySet().stream()
Copy link
Author

Choose a reason for hiding this comment

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

제가 Map - entrySet에 대한 이해도가 높지않아 해당 부분에서 Gpt의 도움을 받았습니다 😅
결과를 단순 조회할 땐 .forEach(), 값을 연산할 땐 .stream()을 사용하는 걸 권장한다고 하여 이렇게 작성했는데
바보같은 질문일 수도 있을 것 같지만.. 성능적으로는 이렇게 구현하는 게 권장될 수 있지만 .stream() 등을 잘 모르는 사람이 봤을때
같은 matchResults를 다른 메서드를 통해 조작한다는 게 통일성 없어보이거나 이해가 어렵지 않으려나? 가독성이 별로지않나? 라는 개인적인 생각을 했습니다ㅎㅎ 아무래도 기능/성능적 부분이 우선일까요? 의견을 공유받고 싶습니다!_!

Choose a reason for hiding this comment

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

성능이라고 한다면 어느 정도의 성능을 보장하는 것을 목표로 하시나요?

컴퓨터는 생각보다 매우 빠르고, 자바 또한 성능이 느리다는 말은 옛말이 된지 오래에요.
(제가 가진 M1 맥북은 160억 개의 트랜지스터가 있고, 초당 11조 번의 연산력을 제공한다고 해요)

그렇다면 지금 구현해 주신 로또 게임이 성능을 신경 써야 할 정도의 연산이 필요할까요?

그게 아니라면 무엇을 선택해야 할까요?

Copy link
Author

Choose a reason for hiding this comment

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

관련 링크 감사합니다!_! 😃
성능을 크게 고려하지 않아도 되는 부분이면, 가독성이 우선이 될 것 같아. stream을 사용하는 게 좋을 거란 생각이 듭니다.

Copy link

@seokjin8678 seokjin8678 left a comment

Choose a reason for hiding this comment

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

안녕하세요 예진님! 로또 미션 2단계 구현하느라 고생하셨습니다!

모든 원시 값과 문자열을 포장한다. 라는 요구사항을 잘 지켜주셨는데, 그에 따라 생겨난 객체들이 그저 "값"으로만 사용되는 것 같아요.

객체가 자율적으로 행동하는 존재가 아닌, 그저 수동적으로 행동하는 존재 같아요.

조금 비유를 하자면, 객체에 영혼이 없는 것 같네요. 😂

조영호 님의 "객체지향의 사실과 오해"라는 책을 읽어보시면 도움이 될 것 같아요.

얇은 책이긴 하나, 읽는 데 시간이 걸린다면 인터넷에 정리해둔 자료가 많을거에요.

그리고 테스트 코드가 없는데, 일부 메서드를 보니 테스트 할 수 있는 곳이 보여요.

해당 사항 반영해 주시고, 재요청 부탁드립니다!

Comment on lines 9 to 11
public Lotto(List<Integer> numbers) {
this.numbers = new LottoNumbers(numbers);
}

Choose a reason for hiding this comment

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

Suggested change
public Lotto(List<Integer> numbers) {
this.numbers = new LottoNumbers(numbers);
}
public Lotto(LottoNumbers LottoNumbers) {
this.numbers = LottoNumbers;
}

생성자의 인자로 LottoNumbers이 아닌, List<Integer> 타입을 받으신 이유가 있으실까요?

Copy link
Author

Choose a reason for hiding this comment

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

앗 수정했습니다🤣🤣

Comment on lines 11 to 14
public LottoNumbers(List<Integer> numbers) {
validate(numbers);
this.numbers = numbers;
}

Choose a reason for hiding this comment

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

LottoNumbers는 로또 번호에 대한 지식을 가지고 있군요!

그 지식 중 하나는 "반드시 6개의 번호를 가진다"에요.

즉, LottoNumbers는 필드로 가지는 List<Integer>에 6개의 숫자가 반드시 포함되어야 해요.

이 규칙은 절대로 변하면 안 되는 도메인 규칙이에요.
(또는 다른 나라의 로또를 구현해야 하거나, 우리나라의 로또의 규칙이 달라지거나 하면 변할 수 있겠지만..)

하지만 지금 구현에서는 정말 "6개"의 숫자가 변경되지 않는다고 보장할 수 있을까요?

List<Integer> numbers = new ArrayList();
LottoNumbers lottoNumbers = new LottoNumbers(numbers);
numbers.clear();

assertThat(lottoNumbers.getNumbers()).hasSize(6);

Copy link
Author

Choose a reason for hiding this comment

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

거기까지 방어해야한단 생각을 못했던 것 같아요!!
민지님의 코드를 참고해 this.numbers = List.copyOf(numbers);로 수정하여 추후 변경이 불가하도록 했습니다

Comment on lines 8 to 33
public class LottoGenerator {

private static final int MAX_NUMBER = 45;
private static final int LOTTO_SIZE = 6;

public List<Lotto> generate(int count) {
return IntStream.range(0, count)
.mapToObj(i -> new Lotto(pickFirstSixSorted(createShuffledNumbers())))
.collect(Collectors.toList());
}

private List<Integer> createShuffledNumbers() {
List<Integer> numbers = IntStream.rangeClosed(1, MAX_NUMBER)
.boxed()
.collect(Collectors.toList());
Collections.shuffle(numbers);
return numbers;
}

private List<Integer> pickFirstSixSorted(List<Integer> numbers) {
return numbers.stream()
.limit(LOTTO_SIZE)
.sorted()
.collect(Collectors.toList());
}
}

Choose a reason for hiding this comment

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

로또 번호를 생성해 주는 클래스군요!

해당 클래스에서 생성된 로또 번호는 1~45 사이의 중복되지 않은 값 6개 라는 것을 알 수 있어요.

하지만 그 의미는 해당 클래스 내에서만 유효하고, 해당 클래스를 사용하는 곳에서는 그 의미를 찾을 수 없어요.

LottoGenerator lottoGenerator = new LottoGenerator();
List<Integer> lottoNumbers = lottoGenerator.generate();

lottoNumbers.add(1);

그렇다면 해당 클래스를 사용하는 곳에서도 그 의미를 어떻게 전달할 수 있을까요?

Copy link
Author

Choose a reason for hiding this comment

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

List<Integer> 대신 의미를 가진 LottoNumbers객체를 생성하면 의미가 명확할 것 같습니다

@@ -0,0 +1,20 @@
package lotto.model;

public class Money {

Choose a reason for hiding this comment

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

int money = 1000 대신 Money money = new Money(1000)과 같이 사용하면, 명확하게 돈이라는 의미를 가지게 할 수 있겠군요!

또한 내부 필드가 final으로 지정되어, 여러 스레드에서 공유해도 안전할 것 같네요!

이러한 객체 구현을 "값 객체(Value Object)"라고 불러요.

값 객체는 상태를 변경 메서드를 제공하지 않으며, 그 자체로 식별성을 가져요.

하지만 지금 구현에서도 그럴까요?

Money first = new Money(100);
Money second = new Money(100);

assertThat(first).isEqualTo(second);

Copy link
Author

Choose a reason for hiding this comment

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

감사합니다 수정했습니다 !!

Comment on lines 19 to 31
public void printWinningStatistics(int money, Map<String, Long> winningLottos) {
System.out.println("당첨 통계");
System.out.println("---------");
System.out.println("3개 일치 (5000원) - " + winningLottos.get("3") + "개");
System.out.println("4개 일치 (50000원) - " + winningLottos.get("4") + "개");
System.out.println("5개 일치 (1500000원) - " + winningLottos.get("5") + "개");
System.out.println("6개 일치 (2000000000원) - " + winningLottos.get("6") + "개");

Long totalWinningMoney = winningLottos.get("total");

double profitRate = (double) totalWinningMoney / money;
System.out.printf("총 수익률은 %.2f%%입니다.%n", profitRate);
}

Choose a reason for hiding this comment

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

해당 메서드에서는 winningLottos Map에 "3", "4", "5", "6", "total"이라는 Key가 있다는 것을 알 수 있어요.

하지만 외부에서는 이런 Key가 있다는 것을 알 수 있을까요?

Copy link
Author

@yaevrai yaevrai Jun 1, 2025

Choose a reason for hiding this comment

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

enum을 사용하여 좀 더 명시적일 수 있게 개선해보았습니답

public class LottoController {

public static void main(String[] args) {
LottoService service = new LottoService(new LottoInputView(), new LottoOutputView());

Choose a reason for hiding this comment

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

예진님이 생각하시는 Controller의 역할은 무엇인가요? 그렇다면 Service의 역할은 무엇인가요?

Comment on lines 28 to 42
Money purchaseMoney = new Money(inputView.inputMoney());

List<Lotto> lottos = purchaseLottos(purchaseMoney);

LottoTicket lottoTicket = purchaseMoney.calculateTicketCount();

outputView.printPurchasedLottoCount(lottoTicket.getCount());
outputView.printLottos(lottos);

WinningNumbers winningNumbers = new WinningNumbers(inputView.inputWinningNumber());

WinningResult winningResult = calculateWinningResult(lottos, winningNumbers);

outputView.printWinningStatistics(purchaseMoney.getValue(),
winningResult.getWinningStatistics());

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.

outputView는 똑같은 출력이랑 붙였고 나머지는.. 큰 기준없이 띄웠는데 가독성이 떨어지나요?? 더 나은 방향이 있을까요?😅


List<Lotto> lottos = purchaseLottos(purchaseMoney);

LottoTicket lottoTicket = purchaseMoney.calculateTicketCount();

Choose a reason for hiding this comment

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

LottoTicket이 정말 필요한 객체일까요?

요구사항에서는 모든 원시값을 포장하라고 했어요.

그렇다고 프로젝트에 사용되는 모든 원시값을 포장하면 어떻게 될까요?

Comment on lines 5 to 7
public class Lotto {

private final LottoNumbers numbers;

Choose a reason for hiding this comment

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

Lotto, LottoNumbers 둘의 차이는 뭔가요? 각자 어떤 역할을 맡고 있나요?

Copy link
Author

Choose a reason for hiding this comment

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

하나의 로또티켓이 있고, 이 티켓이 보유한 번호 집합을 LottoNumbers로 갖고 있도록 했는데요,
로또 티켓 안에 로또 번호가 여러개일 수 있는 점을 생각해보면, 포장이 명확하지 않았던 것 같아 아래와 같이 수정했습니다.

public class Lottos {

    private final List<LottoNumbers> lottoNumbers;

matchResults.forEach((matchCount, count) ->
statistics.put(String.valueOf(matchCount), (long) count));

long totalPrize = matchResults.entrySet().stream()

Choose a reason for hiding this comment

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

성능이라고 한다면 어느 정도의 성능을 보장하는 것을 목표로 하시나요?

컴퓨터는 생각보다 매우 빠르고, 자바 또한 성능이 느리다는 말은 옛말이 된지 오래에요.
(제가 가진 M1 맥북은 160억 개의 트랜지스터가 있고, 초당 11조 번의 연산력을 제공한다고 해요)

그렇다면 지금 구현해 주신 로또 게임이 성능을 신경 써야 할 정도의 연산이 필요할까요?

그게 아니라면 무엇을 선택해야 할까요?

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