Skip to content

Conversation

@Mo-bile
Copy link

@Mo-bile Mo-bile commented Nov 30, 2025

안녕하세요?
피드백 요청드립니다 👍

객체가 잘 나누어져서인지 수동입력의 변경 여파가 로또 자동생성 및 당첨 계산, 수익률 계산에 코드수정이 거의 없어서 뿌듯했습니다.
크게는 LottoGame 외 별도의 LottoBuy라는 흐름을 새롭게 만들었는데 이렇게 로또 입력과, 계산 크게 두개 비즈니스 흐름을 만들었습니다.
이렇게 접근해도 되는건지 고민을 하다가 시도했습니다.

잘 부탁드리겠습니다.
감사합니다 👍

Copy link
Contributor

@javajigi javajigi left a comment

Choose a reason for hiding this comment

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

객체가 잘 나누어져서인지 수동입력의 변경 여파가 로또 자동생성 및 당첨 계산, 수익률 계산에 코드수정이 거의 없어서 뿌듯했습니다. 이 부분을 제대로 느꼈군요. 👍
객체 설계를 잘 했을 때, 유지보수하기 좋은 코드를 구현했을 때 이 같은 느낌을 받을 수 있습니다.
객체의 의존관계를 끊고, 독립적으로 구현했을 경우 얻을 수 있는 효과입니다.
로또 생성과 관련해 다른 접근 방식으로 고민해 봤으면 하는 바람으로 피드백 남겨요.

피드백 반영하면서 시간날 때 '수강신청' 미션의 1단계 - 레거시 코드 리팩터링를 병행해 볼 것을 추천합니다.

@Mo-bile
Copy link
Author

Mo-bile commented Dec 1, 2025

안녕하세요?
요청 주신 바 대로 인터페이스로 이용해보았습니다.

LottoBuy 의 생성자 부분을 분리해서 각 인터페이스의 메서드로 분리하였습니다.
그리고 각 구현체(자동, 수동, 혼합)으로 구현하여 각자의 방식으로 생성하였습니다.
저에게 의도하신 방식이 이 방식인지 조금 고민이 남아요

감사합니다!

Copy link
Contributor

@javajigi javajigi left a comment

Choose a reason for hiding this comment

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

인터페이스 구현과 관련한 피드백 하나 남겼으니 도전해 보면 좋겠습니다.
로또 마지막 단계인만큼 이 연습을 하면서 "수강신청 - 레거시 코드 리팩터링" 미션의 "1단계 - 레거시 코드 리팩터링" 미션 병행할 것을 추천합니다.
미션을 병행하는 만큼 이 피드백 반영에 조급해 하지 말고 충분히 여유를 가지고 고민해 봤으면 좋겠습니다.

Comment on lines 36 to 44
LottoGenerator lottoGenerator = new LottoCombineGenerator();
if(isAllAutoGenerate(manualCount)) {
lottoGenerator = new LottoAutoGenerator();
}
if(isAllManualGenerate(manualCount, pay)) {
lottoGenerator = new LottoManulGenerator();
}
List<String> manualLottoNumbers = readManualLottos(manualCount);
LottoBuy lottoBuy = lottoGenerator.generate(pay, manualLottoNumbers);
Copy link
Contributor

Choose a reason for hiding this comment

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

인터페이스 기반으로 구현하면서 오히려 복잡도가 높아진 느낌이 든다.
인터페이스 기반으로 구현했을 때의 테스트 예시이다.
아래와 같이 구현하기 위해 도전해 보는 것은 어떨까?

아래 예시에서 LottoBundleGenerator는 LottoCombineGenerator와 같은 역할
Lottos는 LottoTickets와 같음

class LottoBundleGeneratorTest {
    @Test
    void generate() {
        Money money = new Money(5_000);
        List<String> values = List.of(
                "1,2,3,4,5,6",
                "2,3,4,5,6,7",
                "3,4,5,6,7,8"
        );
        LottoGenerator lottoGenerator = new LottoBundleGenerator(money, values);
        Lottos lottos = lottoGenerator.generate();
        assertThat(lottos.size()).isEqualTo(money.countOfBuyingLotto());
        System.out.println(lottos);
    }
}

@Mo-bile
Copy link
Author

Mo-bile commented Dec 7, 2025

안녕하세요
인터페이스 분리 관련 리뷰 요청드립니다 👍

인터페이스에 대한 부족한 이해로 인터페이스 분리 방식이 복잡성을 가중시켰던 것 같습니다
짚어 주신 점 감사드립니다.

고민이 있습니다.
LottoGenerator 의 3가지 구현체 중 LottoAutoGenerator는 필드가 다른 두개의 구현체와 달리 manualLottoNumbers는 없고 pay 만있는데요
LottoAutoGenerator 는 자동생성이므로 manualLottoNumbers 가 필요없더라도, 클라이어언트 코드 입장에서는 구현체만 갈아끼워야하니 manualLottoNumbers 필드를 마찬가지로 추가해야할까요?

Copy link
Contributor

@javajigi javajigi left a comment

Choose a reason for hiding this comment

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

질문과 관련한 내용은 코드 리뷰 피드백을 통해 고민해 봤으면 합니다.
LottoGenerator가 로또를 생성하는 것이 아니라 LottoBuy(수동/자동과 관련있는)를 생성하다보니 오히려 복잡도가 높아지는 것 같아요.
LottoGenerator가 로또의 목록을 생성하도록 구현한다면 훨씬 더 단순하게 구현할 수 있지 않을까요?


public interface LottoGenerator {

LottoBuy generate();
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
LottoBuy generate();
LottoTickets generate();

LottoBuy를 생성해 반환하다보니 Auto인 경우에도 Manual, Manual인 경우에도 Auto 값을 가짐
각각의 Generator 구현체에서 바로 LottoTickets를 생성하도록 접근해 본다.
현재 구현과 어떻게 다른지 고민해 본다.

Copy link
Author

Choose a reason for hiding this comment

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

LottoBuy 로 생성 후 반환하려고 하다 보니 LottoBuy의 생성자 틀에 억지로 맞추어야했다.
그래서 Manual 인 경우에도 Auto 값을 가지게 되었다
그에 맞춰 각각 구현체의 필드도 모두 같아야한다고 생각하게 되었다.


public class LottoManulGenerator implements LottoGenerator {

private final int pay;
Copy link
Contributor

Choose a reason for hiding this comment

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

pay는 몰라도 되지 않을까?
manualLottoNumbers 값만 있으면 수동 로또 생성이 가능하다.

@Mo-bile
Copy link
Author

Mo-bile commented Dec 9, 2025

안녕하세요? 피드백 요청드립니다!

우선 감사드립니다 👍
스스로 고민할 수 있는 부분을 주신 덕분에 피드백 해주신 부분을 반영하면서 제가 작전에 드린 질문에 대해서 스스로 깨달음을 얻게 되었습니다.

현재 상태가 복잡하다면, 여러 객체 중 이 부분이 꼭 필요할까 곧바로 가능하지 않을까? 이런 고민도 함께 하겠습니다

모호한 인터페이스에 대한 이해가 점점 잡혀갑니다. 💯

Copy link
Contributor

@javajigi javajigi left a comment

Choose a reason for hiding this comment

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

아마도 로또의 마지막 피드백이 될 것 같네요.
LottoAutoGenerator, LottoManualGenerator와 같은 객체가 이미 있는데 Auto, Manual과 같은 객체는 없어도 되지 않을까라는 생각이 들어 피드백 남겨 봅니다.
각각의 객체의 역할과 책임에 대해 고민해 보면 어떨까요?

import java.util.stream.Stream;
import lotto.domain.model.*;

public class LottoCombineGenerator implements LottoGenerator {
Copy link
Contributor

Choose a reason for hiding this comment

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

LottoAutoGenerator, LottoManualGenerator와 같은 객체가 있는데 굳이 Auto, Manual과 같은 객체는 없어도 되지 않을까?
아래 코드의 Lottos는 재영님 코드의 LottoTickets와 같은 역할을 함

이와 같이 인터페이스를 구현하고, 구현체가 서로 연결되어 있는 경우 아래와 같은 구현체를 통해 구현체를 통합할 수 있음.
보통 디자인 패턴에서 컴포지트 패턴으로 알려져 있음.
아래와 같은 객체 추가에 따른 효과를 느껴 봤으면 하는 바람으로 피드백 남겨봄.

public class LottosBundleGenerator implements LottosGenerator {
    private final List<LottosGenerator> lottosGenerators;

    public LottosBundleGenerator(Money money, List<String> manualLottoText) {
        this(toLottosGenerators(money, manualLottoText));
    }

    private static List<LottosGenerator> toLottosGenerators(Money money, List<String> manualLottoText) {
        // manualLottoText 활용해 ManualLottosGenerator 생성
        // money에서 수동으로 구매한 로또 수 만큼 차감한 후 AutoLottosGenerator 생성
    }

    public LottosBundleGenerator(List<LottosGenerator> lottosGenerators) {
        this.lottosGenerators = lottosGenerators;
    }

    @Override
    public Lottos generate() {
        // List<LottosGenerator> 반복문 돌며 로또 생성
        return  로또를 합쳐서 반환;
    }
}

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