[2단계 - 로또(수동)] 유콩(김유빈) 미션 제출합니다.

[kyukong]

안녕하세요 소니
로또 2단계 미션 제출합니다.

2단계 브런치를 새로 생성하는 과정에 문제가 있어서
이전 이력을 그대로 가져오느라 커밋 시간이 맞지 않은 점 양해부탁드려요.🥲

---

깨달은 점

생성자 파라미터 타입

이전까지는 기능을 구현하면서 현재 상황에 맞는 타입 을 생성자 파라미터로 만들었었습니다. Input 으로 받아온 값들은 모두 String 으로 받았으며 Lotto 도 로또 번호들을 입력으로 받아왔기 때문에 List의 String 또는 List의 Integer 등으로 받아왔습니다. 그러나 객체가 가지는 인스턴스 변수의 타입과는 다른 타입의 파라미터를 받는다면 해당 타입과는 다른 검증 과정과 변환 과정이 필요합니다. 이후에 기능이 추가되면서 구현된 타입에 맞지 않은(String 파라미터를 갖는 생성자를 만들었는데 Integer 로 생성자를 호출하고 싶음) 값으로 생성자를 호출하고 싶다면 추가 변환 및 검증이 필요했고 더 확장될수록 대체 이 객체의 생성자 기준은 어떤 것인지가 파악하기 어려웠습니다.

소니 피드백을 받고 생각해본 결과 객체의 생성자 파라미터 기본값은 인스턴스 변수로 가지는 타입으로 두는 것이 적절하다고 느꼈습니다. 하나의 타입 기준을 정해두니 여러 곳에서 생성자를 호출해도 그때 상황에 맞게 변환 후 생성자를 호출할 수 있었으며 생성자 내 검증 범위가 단순화되었습니다. 또 기본 타입으로 생성자를 호출하는 것이 도메인 내부 로직에 대한 수정을 줄일 수 있다고 생각합니다.

---

1단계 피드백 답변

VO 사용 이유

소니가 언급해주기 전까지 VO를 몰랐어서 찾아봤습니다. 🤣  equals를 오버라이딩한 이유와 마찬가지로 로또 번호를 번호가 동일한 객체를 다른 객체로 분류하는 것이 의미가 없다고 생각했기 때문입니다. 동일한 로또 번호로 구성되어 있는 로또는 각각 다른 개체로써 당첨 번호와 비교가능한 유일한 객체지만 로또 번호는 로또를 생성하기 위해 사용되는 수단입니다. 로또 번호라는 특정한 번호의 공이 여러번 사용 되어 로또라는 객체를 구성하는 역할일 뿐 1번과 또 다른 1번이 동일한 객체로써 사용되기 때문에 Ball 을 VO로 만들었습니다.

---

1단계 피드백 질문

Lotto 변수의 필요성

public class Lotto implements Comparable<Lotto> {
    private static final List<Integer> lottoTotalNumbers = new ArrayList<>();
    private static final int BALL_COUNT = 6;
    public static final int LOTTO_PRICE = 1000;
    private static final String ERROR_BALL_COUNT = BALL_COUNT + "개의 숫자를 입력해주세요";
    private static final String ERROR_DUPLICATED_NUMBER = "번호가 중복됩니다!";

    private final List<Ball> lotto = new ArrayList<>();

지난번 1단계에서 다음과 같은 피드백을 받았습니다.

아래 두 필드 모두 필요한걸까요?

• List lottoTotalNumbers = new ArrayList<>()
  소니의 피드백대로 로또 번호 에 대한 캐싱이므로 Ball 로 이동하였습니다.

• List lotto = new ArrayList<>();
  Lotto 라는 객체에 포함되어 있는 Ball 들의 묶음이기 때문에 필요하다고 생각합니다...🥺 Lotto 객체 내에 자신의 정보를 담고 있음으로써 자동 로또 출력 시 정보를 반환할 수 있고 WinningLotto 객체 생성 시 당첨 번호와 보너스볼 중복 여부를 확인하는 기능을 제공할 수 있다고 생각합니다. 아닐까요?😔
  (아래 코드에도 설명을 붙이겠지만 TreeSet 타입으로 수정하였습니다!)

---

지난번 피드백에 대한 글이 많다보니 어느새 이렇게 길어졌네요.😅
이번 리뷰도 잘 부탁드립니다!🙇‍♀️

[sonypark]

유콩 안녕하세요!
2단계 구현하느라 고생하셨습니다.
대부분 잘 구현해주셨습니다 👍
몇가지 코멘트 남긴 부분만 확인 부탁드립니다!

Lotto 변수의 필요성

List lottoTotalNumbers = new ArrayList<>() -> 이 부분은 필요없을 것 같아 코멘트 남긴 것이었습니다. 🙂 로또번호를 가진 컬렉션은 필요합니다! ㅎㅎ

[kyukong]

Controller 관련

컨트롤러를 유틸성 클래스처럼 static 메서드로 만드신 이유가 있나요?

지난 미션 리뷰어님께 컨트롤러 내에서 인스턴스 변수를 사용해도 되는지에 대해 여쭈어 본적이 있습니다. 답변은 Controller 의 역할은 사용자의 요청에 따라 도메인과 뷰를 이어주기 때문에 단순히 흐름만을 제어하는 역할로 매번 객체를 생성할 필요가 없어 보인다. 또한 Controller 내에서 인스턴스 변수를 생성할 경우 멀티쓰레드 환경에서 여러 쓰레드가 인스턴스 변수에 접근하여 예상치못한 동작이 발생할 수 있다. 라고 해주셨습니다. (답변을 요약한 내용입니다. 😄)

각 인스턴스마다 다른 객체로 분류되는 도메인과는 달리 Controller 는 도메인의 기능을 제공하기 위해 거쳐가는 중간 과정의 역할을 합니다. MVC 구조에서 객체 접근 흐름을 본다면 여러 사용자(쓰레드)가 하나의 Controller 를 공유하여 각각의 도메인 객체 및 뷰를 호출하여 서비스를 제공받는다고 이해했습니다.

혹시나 잘못 이해한 부분이 있다면 피드백 바랍니다. 🥲

[kyukong]

소니! 1단계부터 나름 코드에 설명을 덧붙였지만 대답이 돌아오지 않아 이상해서 찾아보니 댓글이 제대로 달리지 않았던 것 같습니다..! 맞나요?
(고요속의 외침을 하고 있었던 것 같습니다!ㅋㅋ)
대부분은 코드에 대한 의도를 작성했었으므로 생략하겠습니다.🤣

Lotto 의 인스턴스 변수 lotto 의 타입 변경 (ArrayList -> TreeSet)

Lotto 의 인스턴스 변수 lotto 를 Set 으로 바꿀지말지 1단계부터 고민이 많았는데 수정해보았습니다. 다른 개발자가 변수의 타입만봐도 특정한 기준으로 정렬이 되어있으며 중복되지 않음을 명시적으로 보여줄 수 있다고 생각해 바꾸어보았습니다. 생성자 파라미터로는 그대로 List 를 받는데 그건 사용자에게 적절한 오류 메시지를 보여주기 위해 중복의 경우를 알아야하기 때문에 그대로 두었습니다.
어차피 검증과정이 거치기 때문에 그대로 ArrayList 를 사용하는 것이 좋을까요? 아니면 명시적으로 용도를 알리는 것이 좋을까요?

[kyukong]

Creator 로또 생성 관련

    public static Lottos createManualLottos(final LottoCount lottoCount) {
        List<Lotto> manualLottos = new ArrayList<>();
        for (int i = 0; i < lottoCount.getManualCount(); i++) {
            manualLottos.add(createLottoNumbers());
        }
        return LottoGenerator.pickManualLottos(manualLottos);
    }

    public static Lottos createAutoLottos(final LottoCount lottoCount) {
        return LottoGenerator.pickAutoLottos(lottoCount);
    }

    public static Lotto createLottoNumbers() {
        try {
            return new Lotto(inputLottoNumbers(entering));
        } catch (IllegalArgumentException error) {
            printErrorMessage(error.getMessage());
            return createLottoNumbers();
        }
    }

이 친구들은 View 관련 로직도 없고 LottoCount를 받아 Lottos를 생성하는 역할을 하고 있네요.
공통점이 많아 클래스로 추출하기 좋아 보이는데 어떠신가요? 로또 생성을 담당하는 도메인 클래스를 만들어보는건 어떤가요?

질문에 대한 의도를 정확하게 파악하지 못해 아래 코드 의도를 덧붙였습니다! 🤣

지난번 1단계 피드백을 받고 요구사항이 늘어나는 것을 대비해서 로또를 생성하는 객체 LottoGenerator 를 만들었습니다. 해당 객체는 수동 로또와 자동 로또 Lottos 를 생성합니다. 수동 로또는 사용자의 입력(view)을 받아 그 값을 토대로 로또를 생성하기도 하고 로또(번호 6개)를 입력받는 지난 주 당첨 번호의 입력 메소드를 활용할 수 있다고 생각해 Creator 에 두었습니다. 하나의 로또를 입력받는 createLottoNumbers 메소드로 구매하고 싶은 수동 로또 개수만큼 입력받아 LottoGenerator 에게 넘겨줍니다.

자동 로또는 그저 자동 로또 개수를 입력받아 LottoGenerator 내부에서 랜덤 번호 추출 후 생성하지만 Creator 는 Controller 에서 원하는 객체를 반환해주는 역할을 하므로 통일성을 위해 Creator 에 두었습니다. view 의 개입이 없으므로 자동 로또만 위치를 달리해도 괜찮을까요?

로또를 생성하는 객체 LottoGenerator 를 말하는 것이 맞을까요? 아니면 잘못 이해한걸까요? 😅
(해당 부분은 아직 수정하지 않았습니다!)

[sonypark]

유콩 안녕하세요. 리뷰가 늦어서 죄송합니다😭
질문해주신 내용에 대한 제 생각 말씀드리겠습니다 :)

Controller 관련

네 현재 어플리케이션 상에서 컨트롤러는 main 메서드와 차이점은 없는 것 같습니다. run 메서드 안에서 모든 로직을 호출하고 어플리케이션이 종료되는 구조인데요. main 메서드에서 수행해도 될 로직을 컨트롤러라는 레이어를 하나 더 만들어 수행한것처럼 보입니다. 말씀하신것처럼 컨트롤러는 View와 Domain 레이어의 중간에서 서로를 이어주는 역할을 합니다. 컨트롤러는 말씀하신 문제점 때문에 인스턴스 변수를 갖지 않지만 서비스에 의존해 해당 서비스를 생성자로 주입 받아 사용되곤 합니다. new LottoController(LottoService). 이 부분은 나중에 스프링 MVC, 의존성 주입 등을 공부해보시면 좋을 것 같습니다. 정리하자면 지금처럼 사용해도 문제는 없지만 'main 메서드에서 할 일을 컨트롤러라는 레이어를 하나 더 만들어서 수행할 필요가 있을까?'입니다.(이 부분은 수정 요청 사항은 아닙니다😊)

Creator 로또 생성 관련

수동 로또를 생성하는 부분이 InputView랑 강하게 결합되어 있군요!
for문 안에 InputView에서 숫자를 받아 Lotto를 생성하는 로직이 결합되어 있는데 이를 분리해보면 어떨까요?
(ex. InputView에서 사용자가 입력한 수동로또숫자를 리스트 형태로 받아 LottoGenerator에 전달하면 LottoGenerator에서 수동,자동 로또를 생성하는 방식)
그렇게 되면 view로직과 도메인 로직을 분리돼서 유연한 구조가 되고 책임이 명확해질 것 같아서요.

[kyukong]

Lotto 생성자 파라미터 관련

2단계 PR 을 보낼 때 생성자 파라미터 일치에 대한 글을 적었으면서 바로 다르게 입력받았네요. 😅 정적 팩토리 메서드의 네이밍 컨벤션 중 from 을 사용하여 내부에 형변환이 일어날 수 있음을 알리는 1번 방법으로 선택했습니다. 좋은 아이디어 감사합니다!

[sonypark]

유콩 안녕하세요~
로또 미션은 이만 머지하겠습니다!
궁금한점에 대해 질문 많이 주신것 감사해요!
다음 미션도 응원합니다 🙌🏻

[sonypark]

이 부분 테스트가 없는것 같네요 😅