[로또] 김경수 미션 제출입니다.

[Siul49]

오늘도 리뷰 와주셔서 감사합니다 정말루..

1. 신경쓴 부분

• 객체지향적으로 프로그램을 짜려고 했고
• 의존성 주입에 신경을 좀 썼습니다
• 변수명에도 신경을 썼습니다

2. 피드백 받고싶은 부분

• enum을 잘 사용했는지
• 변수명 바꿀 사항이 있는지
• 의존성 주입이나
• appConfig 잘 사용했는지

3. 고려한 예외 케이스

• 로또 숫자 6개
• 중복되는 숫자가 있는지
• 당첨 숫자 6개이상 입력하진 않았는지

[daeGULLL]

저도 감자라 정확하게 개선점을 드리진 못한것 같지만..ㅎㅎ 저번주에 비해 코드가 굉장히 깔끔해지고 잘 정리된 것 같은 느낌이었습니다! 또 직관적인 이름을 붙인 서비스를 여러개 만드셨던 게 가독성을 좋게 했던 것 같습니다! 저도 이 부분 잘 주워가도록 하겠습니다ㅎㅎ 수고하셨고 마지막 과제 같이 힘냅시다..ㅠㅠ

[daeGULLL]

피드백 요사항에 적어두실 필요 없었던 것 같은데요??ㅎㅎ 깔끔하게 쓰셨어요
그런데 util에 들어있는 validator같은 경우는 여기 의존성 주입에서 빼시고 utility 클래스로 만드시는게 좋을 것 같아용
모든 메서드에 static을 붙이면 굳이 instance를 생성하지 않고도 내부 메서드를 불러올 수 있습니다!!

[seulnan]

appConfig의 역할을 쉽게 설명하면 객체를 생성하는 역할과 실행하는 역할을 분리하는 것이라 validator같은 유틸리티클래스는 객체를 생성하지않고 그냥 개별클래스로 관리하는게 더 좋을듯합니다!!

[seulnan]

SOLID원칙
AppConfig 사용법

[daeGULLL]

지금 로또 게임 프로그램 상에서는 인스턴스를 프로그램 실행 중 재생성해야할 필요가 없으니 final를 앞에 붙여주시는게 더 좋은 시도일 것 같습니다!

[daeGULLL]

변수를 없애시구 그냥 메서드 파라미터로만 무언가를 받을 수 있게 하시는게 더 좋을 것 같습니다!! 변수로 값을 저장하게 되면 실사용하기 위해 getter랑 setter가 생기게 되는데 이 부분은 뷰보다는 모델에 더 가까운 것 같습니다! 차라리 저 부분을 관리하는 모델을 하나 새로 만드시는게 더 좋을 것 같아요!!

밑의 budget 등등 값을 받는 부분의 코드는 변수에 값을 저장하는 것보다는 public (돌려주는 값 타입) 메서드명(){} 이런식으로 설정하시고 처리한 값을 return하는게 더 좋은 방안일 것 같습니다!!

[daeGULLL]

저도 이부분 코드 짤때 고민했는데, 이렇게 내부에서 로또의 getter 함수를 호출하게 되면 모델이랑 뷰의 의존성이 생겨서 안된다구 피드백을 받은 적이 있어서 다른 방법으로 바꾸셔야할 것 같습니다! 저도 다른 방법을 사용했습니다~~

[daeGULLL]

enum도 .values()메서드를 사용하면 각 내부요소들을 순회할수 있더라구요, 이 Three~six message들을 따로 enum화 시켜서 .values()메서드를 사용하시면 for문이나 .forEach문을 사용해 더 깔끔하게 작성하실 수 있을 것 같습니다!

[seulnan]

전체적으로 객체를 여러번 생성하는 코드가 보이네요 AppConfig를 만드는 이유는 객체를 한번만 생성하고 이를 다른곳에서 재사용하기위해서라고 알고있어요! 그러려면 필드에 객체를 저장하고 한번만 초기화하도록 변경해야할 것 같아요!

[seulnan]

import lotto.controller.LottoController;
import lotto.service.AdjustmentLotto;
import lotto.service.CalculateTotalBenefit;
import lotto.service.CountCorrectNumber;
import lotto.service.LottoList;
import lotto.util.Validator;
import lotto.view.UserInput;
import lotto.view.UserOutput;

public class AppConfig {
    private final UserInput userInput = new UserInput();
    private final UserOutput userOutput = new UserOutput();
    private final Validator validator = new Validator();
    private final LottoList lottoList = new LottoList(userInput.getNumberOfLotto()); // 한 번만 생성
    private final CountCorrectNumber countCorrectNumber = new CountCorrectNumber(lottoList.getLottoContainer());
    private final AdjustmentLotto adjustmentLotto = new AdjustmentLotto(countCorrectNumber.getLottoList(), userInput.getBonusNumber());
    private final CalculateTotalBenefit calculateTotalBenefit = new CalculateTotalBenefit(adjustmentLotto.getLotto(), userInput.getBudget());

    public UserInput userInput() {
        return userInput; 
    }

    public UserOutput userOutput() {
        return userOutput; 
    }

    public Validator validator() {
        return validator;
    }

    public LottoList makeRandomLotto() {
        return lottoList; 
    }

    public CountCorrectNumber countCorrectNumber() {
        return countCorrectNumber;
    }

    public AdjustmentLotto adjustmentLotto() {
        return adjustmentLotto; 
    }

    public CalculateTotalBenefit calculateTotalBenefit() {
        return calculateTotalBenefit; 
    }

    public LottoController lottoController() {
        return new LottoController(
                userInput(),
                userOutput(),
                validator(),
                makeRandomLotto(),
                countCorrectNumber(),
                adjustmentLotto(),
                calculateTotalBenefit()
        );
    }
}

[seulnan]

너무 고생많으셨습니다:)

다만, 전체적으로 버그가 발생할 가능성이 있는 코드들이 몇개 보이네요...ㅠㅠ(ex.배열초기화오류, 배열이 직접노출되는 문제 등등) 추가로 의존성관리도 왜 해야하는지 기본적인 본질이 뭔지 좀더 깊게 공부하셔서 제대로 활용하시면 더 좋을 것 같아요

테스트코드도 짜다보면 자신의 로직이 어디가 부족한지 알게 되어서 추천드립니다
++ 실제로 클론받아서 실행해보니 모든테스트에서 오류가 나네요...버그수정&탄탄한 테스트코드작성이 필요해보입니다

[seulnan]

이 코드를 다른 곳에서 실행하면
userInput이 호출되면서 새로운 userInput 객체가 계속 생성되어서 사용자 입력값이 유지되지 않는 문제가 발생할 수도 있어요

[seulnan]

여기에서 AppConfig에서 생성한 LottoList가 아니라 새로 만들어진 LottoList를 사용하고있네요 이러면 데이터 일관성이 깨질 위험이 있지않을까요?

[seulnan]

물론 모든 객체생성을 AppConfig에서 해야하는건아니지만 LottoList 같은 핵심객체는 한번만 생성해서 공유하는게 AppConfig를 제대로 활용하는 것 같습니다!