Step4 - 로또(수동)

[ho-jun97]

로또 step4 리뷰 요청드립니다. 잘 부탁드립니다.

LottoGroup 을 수정하면서 부생성자가 늘어나고, 그에 따라 오버로딩 함수 또한 늘어나고 있는데 괜찮은건지 궁금합니다.

[javajigi]

3단계까지 객체 설계를 잘한 덕분인지 큰 변화 없이 4단계 미션 진행했네요. 👍
질문한 바와 같이 LottoGroup의 책임이 많아지고 있는 느낌이 들어 피드백 남겨 봅니다.
다른 부분보다 로또 생성과 관련한 부분을 어떻게 분리할 것인지 고민하는 시간이 되었으면 좋겠습니다.

[javajigi]

중복 체크를 위해 Set으로 변환하고 있다.
Lotto 필드인 List를 Set로 구현해 보면 어떨까?
Set으로 관리하다 외부에서 이 값을 접근할 때 순서를 보장해야 한다면 정렬해서 반환하는 것은 어떨까?

[javajigi]

LottoGroup이 구매한 로또와 관련한 로직을 구현하는 책임과 로또 생성하는 책임을 가지는 것으로 보여진다.
기능이 추가되면서 로또 생성하는 책임의 비율이 높아지고 있는데 로또 생성에 대한 책임을 다른 객체로 분리하는 것은 어떨까?

3단계에서 4단계로 요구사항이 변경될 때 로또를 생성하는 부분의 요구사항만 변경됐다.
로또를 생성하는 부분을 다음과 같은 구조의 인터페이스로 분리해 보는 연습을 해보면 어떨까?
이와 같이 인터페이스로 구현했을 때의 잇점에 대해 고민해 보는 시간을 가져본다.

public interface LottosGenerator {
    Lottos generate();
}

[javajigi]

Suggested change

	Lotto lotto = new Lotto("1", "2", "3", "4", "5", "6");
	Lotto lotto = new Lotto("1,2,3,4,5,6");

문자열 가변인자보다 위와 같이 구현할 수 있으면 더 좋지 않을까?

[ho-jun97]

피드백 반영했습니다.

1. set으로 구현하였고, 외부에서 접근하지 않아서 정렬하고 반환하는 부분은 만들지 않았습니다.

2. LottoGroup 로또 생성하는 부분을 구현체로 구현하였습니다. 구현체로 구현했을 때 이점을 아래와 같이 생각했습니다.

• 로또를 생성하는 부분이 새로 생기거나 변경했을 때 LottoGroup을 건들리지 않아도 된다.
• 책임이 분리 되어 단일 책임 원칙을 지키게 되어 유지보수하는데 좋을거 같습니다.

질문

포비님께서 생각하시는 interface로 구현했을 때 이점이 무엇이라 생각하시는지 궁금합니다.

[javajigi]

인터페이스 기반으로 구현 👍
MixedLottoGenerator까지 구현한 점이 인상적이네요.
로또 미션은 어차피 마지막 단계라 지금 단계에서 한 단계 더 나아간 경험을 해봤으면 하는 바람으로 피드백 남겨 봅니다.

이 피드백 천천히 반영하면서 "수강신청 - 레거시 코드 리팩터링" 미션의 1단계 병행해 보시죠.

[javajigi]

👍

[javajigi]

Suggested change

	int cnt = money.getBuyableCount();
	List<Lotto> lottoArray = new ArrayList<>();
	for (int i = 0; i < cnt; i++) {
	List<Lotto> lottoArray = new ArrayList<>();
	for (int i = 0; i < money.getBuyableCount(); i++) {

저는 굳이 불필요한 로컬 변수 사용하지 않는 방식을 선호함
한번 고려해 보면 어떨까?

[javajigi]

👍

[javajigi]

수동과 자동 로또를 생성하는 구현체를 만들고 아래와 같은 구현체를 통해 구현체를 통합할 수 있음.
보통 디자인 패턴에서 컴포지트 패턴으로 알려져 있음.
아래와 같은 객체 추가에 따른 효과를 느껴 봤으면 하는 바람으로 피드백 남겨봄.

public class LottosBundleGenerator implements LottosGenerator {
    private final Money money;
    private final List<String> manualLottoText;

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

    @Override
    public Lottos generate() {
        // ManualLottosGenerator 활용해 수동 로또 생성
        // AutoLottosGenerator 활용해 자동 로또 생성(수동 로또 수 만큼 Money 차감)
        return 두 로또를 합쳐서 반환;
    }
}

인터페이스 기반으로 구현할 경우 자동 로또만 구매하는 기능을 지원하다 수동 로또를 생성하는 기능이 추가될 경우 다른 부분의 영향을 주지 않으면서 기능을 확장할 수 있다는 것이 가장 큰 강점일 것 같아요.

[ho-jun97]

커밋메시지를 제가 지난번에 했던 메시지로 커밋을 해버렸습니다...ㅠㅠ 죄송합니다. 양해부탁드립니다

포비님께서 피드백 주신대로

기존에 사용하던 MixedLottoGenerator를 제거하고, 수동 로또 생성을 담당하는 ManualLottosGenerator를 새로 분리한 뒤 기존의 AutoLottoGenerator와 함께 LottosBundleGenerator 내부에서 각각 호출하도록 구조를 변경했습니다.

이렇게 구성하니 MixedLottoGenerator가 가지고 있던 중복 코드들이 사라져 전체 로직이 훨씬 단순해졌고, 책임이 명확하게 나누어져 코드의 가독성이 좋아졌다고 느껴집니다.

또한 수동·자동 생성 로직이 독립된 Generator로 분리되면서 다른 생성 로직을 추가하거나 기능을 확장할 때 기존 코드를 수정하지 않아도 되어 유지보수에도 유리하다고 생각이 드네요.

[javajigi]

피드백 반영 잘 했네요. 💯
프로젝트를 지속하다보면 지속적인 변화가 발생하는 부분이 있는데요.
마지막 단계의 인터페이스 기반으로 개발했을 때의 잇점을 느끼고, 변화가 많은 부분을 인터페이스로 구현하고 확장하도록 구현하는 역량도 쌓아나가기를 바랍니다.
지금까지 로또 미션 진행하느라 넘 수고했어요.
로또 미션은 여기서 마무리할게요.