[2단계 - 로또(수동)] 돌범(조재성) 미션 제출합니다. #451

is2js · 2022-03-02T07:38:05Z

1단계에 비해 많은 것들을 수정하고, 피드백 반영해보았습니다.

제가 드라이버 역할을 해서, 1단계 branch를 fetch안하고 바로 진행하였는데 꼬였나봅니다.ㅠ
conflict를 해결하지 않고 제출한PR이 있으니 참고부탁드립니다!

수정사항

controller에서 도메인 의존성을 없애려고 노력했다.
- controller가 input/outputview의 객체를 사용하도록 노력했다.
servcie -> controller dto로 반환된 데이터를 다시 service가 사용되는 경우가 있어서 -> 결과물이라 할지라도 발급된 수+자동 로또들을 service의 인스턴스 변수로 관리하도록 하였음
- List<Lotto> issuedLotto
enum명칭을 수정(rankPrice -> rankPrize) / enum 정적 팩토리 메서드 findByCount -> of 로 수정
- boolean필드를 제거하고 stream filter로 대체함
- orElseThrow대신 NONE객체를 추가하여 반환하도록 함.
자동차경주와 비슷하게 전략패턴을 사용하여 LottoFactory가 로또를 생성하도록 함.
- 수동이냐 자동이냐를 LottoGenerator 구현체에 따라서 다르게 작동하도록 함.
- 요구사항에 적힌 shuffle()을 활용하기 위해 매 숫자랜덤 -> 캐싱되는 LottoNumber + int를 뽑아서 shuffle하여 랜덤 구현하는 방식으로 변경

피드백 반영내역

toDto()등 도메인 내 dto의존성 삭제 요청 반영
역할나누기
- issueLotto 로또 발급
  - service의 issueLotto() 호출 -> 내부에서 lottoFactory에서 Lott들 발급 -> service에서 concat후 사용함.
  - issueLotto 로또 발급 관련
    - LottoGenerator에서 int List -> LottoNumber List -> Lotto로 반환하는 로직을 작성함.
    - LottoFactory는 Generator구현체를 받아 Lotto 발행 뿐만 아니라 List에 모으는 로직들을 추출해놓음.
  - 의문점 Q1. 로또 발급시 service에 추가된 LottoFactory를 제대로 활용하고 있는지 궁금합니다. lottoFactory라는 클래스가 (1)Lotto만을 발급하는 정적팩토리메서드만 모아놔야하는지 (2) service를 대체해야하는게 아니라 내부에서 사용되는데 혹시 대체하면서 controller에서 사용되어야하는지 등... 궁금합니다! (3) 저의 경우, Lotto만을 발급해주는 Lotto클래스의 정적팩토리메서드 역할뿐만 아니라, LottoGroup으로 수동/자동Lotto발급후 List로 반환하는 것까지 정의해서 Lotto이후의 모든 것들을 만들어내는데 괜찮은지
- calculateProfit 수익률 계산
  - 결과값을 감싸는 클래스(ResultDto)에서 계산하도록 수정함 ->
  - 의문점 Q2. 피드백 해주신 내용이 결과값class에 정의해야하는지, 즉, 따로 클래스를 추출해서 해야하는지 현 상태대로 결과값class(ResultDto)에 로직을 추가하기만 함.
  많은 역할을 담당하고있는것으로 보이는데요, 역할을 좀 나눠보면 어떨까요?
  기존의 LottoFactory라는 이름을 가진 클래스는 로또를 발급하는 역할을 하는 명확한 이름을 가지고있었어요. 로또를 발급하는 역할을 하는 클래스를 하나 두는것으로 로또 발급 역할을 부여하고, 수익률을 계산하는 클래스를 따로 추출해보면 역할을 좀 더 나눌 수 있을 것 같아요.
- run 실행 (이름이 run이지만 실제로 result계산을 하는 과정으로 보여요)
  - 메서드명 변경 : run -> calculateResult
결과값을 담당하는 클래스
- 기존에는 map만 넘겨줘서 outputView에서 수익률계산해줬으나, ResultDto라는 wrapping + controller에 반홚해주는 클래스를 생성하였습니다.
- 의문점 Q3. 결과값 반환할 때, Dto가 아닌 클래스를 생성해서 반환해도 되는지(Result로 생성 -> controller가 의존해도 되나??) 만약 클래스명을 Dto로 명시한다면 getter외에 추가로직이 들어가도 되는지. ResultDto에 수익률계산로직을 포함시켰는데 명칭과 로직이 적절한지

추가 의문점

의문점 Q4. controller에서 inputview로 받는 수동 로또가 0개로 없는 경우, LottoService에 빈리스트를 넘겨주도록 분기하였는데 이게 맞는지 궁금합니다.

private void issueLotto() {
    final int manualCount = getManualCount();
    if (manualCount > MANUAL_LOTTO_IS_NONE_NUMBER) {
        lottoService.issueLotto(getManualLottoWith(manualCount));
        outputView.printLotto(lottoService.getIssuedLotto(), manualCount);
        return;
    }
    lottoService.issueLotto(new ArrayList<>());
    outputView.printLotto(lottoService.getIssuedLotto(), manualCount);
}

- 컨트롤러 내부에서 new LottoNumber() 사용을 LottoService로 감춤 - lastWinLotto, bonusNumber를 컨트롤러로 get해서 run(계산) - service의 인스턴스 변수로서 계산 전까지 초기화해서 가지고 있도록 수정 - 예외시 각각을 재입력 받기 위해 - lastWinLotto 먼저 초기화 -> 마지막 입력 bonusNumber는 파라미터화

- boolean field를 제거하고 filter 하나를 추가하여 SECOND, THIRD를 감별 - NONE 객체 추가

- 만약 반복 횟수가 0이라면, IntStream.collect(Collectors.toList())는 [] 빈 List를 반환해준다.

…t 발급 기능 구현

JunHoPark93

안녕하세요 돌범!
2단계 구현하시면서 고민을 많이 하신거같아요. 고생많으셨어요.
코멘트 몇가지 남겨두었는데요, 현재 도메인으로 묶어볼 수 있는 로직들을 좀 더 고민해보시면 좋을 것 같아요.
하시다가 어려운점 있으시면 말씀주세요!

JunHoPark93 · 2022-03-02T14:40:11Z

src/test/java/domain/MoneyTest.java

+    @DisplayName("구입금액에 맞는 로또 발급 갯수 반환을 확인한다.")
+    @Test
+    void calculateCounts() {


여기 클래스 컴파일이 안되는데 확인부탁드려요!

git conflict 해결 과정에서 코드 중복이 생긴체로 제출되었네요! 잘 확인하겠습니다!

JunHoPark93 · 2022-03-02T14:42:41Z

src/main/java/domain/Lotto.java

+    public List<LottoNumber> get() {
+        return this.lotto;


dto반환부를 잘 수정해주셨네요 👍
메서드명이 조금더 명시적이면 좋을것같아요!

오호.. 이 부분은 제가 단일객체(get개별속성())이후의 Dto에 대해서는ex> list, 일급컬렉션
.get()만 붙여서 통채로 전달하는 것을 자신만의 규칙으로 정했었습니다. Lotto.getLotto() 명칭이 어색해서요!

get()도 적절하지 않은 것 같아 getLotto()로 수정하겠습니다! 혹시 괜찮은 메서드명 추천이 가능할까요 제이!

메서드명 훨씬 나아졌어요!

JunHoPark93 · 2022-03-02T14:52:58Z

src/main/java/domain/LottoFactory.java

+import java.util.Collections;
+import java.util.List;
+
+public class LottoFactory {


의문점 Q1. 로또 발급시 service에 추가된 LottoFactory를 제대로 활용하고 있는지 궁금합니다. lottoFactory라는 클래스가 (1)Lotto만을 발급하는 정적팩토리메서드만 모아놔야하는지 (2) service를 대체해야하는게 아니라 내부에서 사용되는데 혹시 대체하면서 controller에서 사용되어야하는지 등... 궁금합니다! (3) 저의 경우, Lotto만을 발급해주는 Lotto클래스의 정적팩토리메서드 역할뿐만 아니라, LottoGroup으로 수동/자동Lotto발급후 List로 반환하는 것까지 정의해서 Lotto이후의 모든 것들을 만들어내는데 괜찮은지

역할을 정의하기 나름인데요, 로또발급만을 담당하게 해주신다면 지금 구조도 괜찮습니다.

이거는 제가 질문을 잘 이해를 못했는데요, 추가설명을 부탁드려요. 추정하여 남겨놓자면, 현재 service 가 어떤 역할을 하시는걸로 생각하셨나요? 일례로 아래와 같이 작성을 해주셨는데요,

public void issueLotto(final List<List<String>> issuedManualLottoInput) { final List<Lotto> issuedManualLotto = LottoFactory.generateManualLottoGroup(issuedManualLottoInput); this.issuedLotto = addAutoLotto(issuedManualLotto); } private List<Lotto> addAutoLotto(final List<Lotto> issuedManualLotto) { final int autoCount = money.calculateCounts() - issuedManualLotto.size(); validate(autoCount); if (autoCount > AUTO_LOTTO_COUNT_LOWER_BOUND) { final List<Lotto> issuedAutoLotto = LottoFactory.generateAutoLottoGroup(autoCount); return concatLotto(issuedManualLotto, issuedAutoLotto); } return issuedManualLotto; }

로또 팩토리가 로또를 발급하는역할을 한다고 해주셨는데 현재 로또서비스란곳에서도 로또번호를 발급하고있어요. 둘의 역할이 모호한거같아요. 추정컨데 서비스를 사용하는것이 아닌 컨트롤러에서 factory를 사용하는게 어떤가요?라는 질문인거같아요. 저는 불필요한 서비스는 없애도 될것같아요. 만들어야 하는 나름의 이유가 있으시면 코멘트 부탁드려요!

마찬가지로 리팩토링을 하다보면 수정될 수 있을 것 같은데요, 역할을 조금더 나눠보면 좋을 것 같아요.

LottoService쪽에 추가로 코멘트 남겨두겠습니다!

JunHoPark93 · 2022-03-02T14:54:23Z

src/main/java/domain/LottoFactory.java

+        return Collections.unmodifiableList(issuedAutoLotto);
+    }
+
+    private static List<Lotto> AccumulateAutoLottoWithCount(final LottoGenerator lottoGenerator, Count count) {


메서드명 확인부탁드려요 :)

메서드명에 소문자로 시작하도록 cameCase로 수정하였습니다! 감사합니다 제이!

JunHoPark93 · 2022-03-02T14:55:43Z

src/main/java/service/LottoService.java

+    public LottoService(final int money) {
+        this.money = new Money(money);
+    }


돈을 생성자인자로 받으신 이유가있을까요?
아마 만들어주신 팩토리와 서비스라는것을 개선하다보면 없어질 수도 있을 것 같아요!

저는 controller에서 도메인을 생성하거나 사용하면 안되는 줄 알고
conroller에서 받는 족족 -> service로 빨리 넘겨주다보니 -> service에서 없어도 될 상태값 이 늘어났었습니다.ㅠㅠ

controller에서도 도메인을 받아 생성하고, service로부터 응답값으로 받아도 된다고 생각을 바꾼 상태입니다.

이 부분 피드백 해주시면 감사하겠습니다.!

구두로 잠시 말씀드렸던것처럼 중요한 부분(우리가 보호해야하는 부분) 과 중요하지 않은 부분의 의존성을 생각해보면 도메인을 생성하셔도 무방합니다!

JunHoPark93 · 2022-03-02T15:01:13Z

src/main/java/service/LottoService.java

+    }
+
+    public String getProfitOrNotMessage(final ResultDto resultDto) {
+        final double profitRate = money.calculateProfit(resultDto.getPrizeProfit());


수익률을 계산하는것이 Money의 역할인지 다시 고민해보시면 좋을 것 같아요!
수익률을 계산하려면 구매한로또들, 당첨로또번호가 필요할텐데 이 둘을 받는 클래스를 만들어보시면 역할분리를 하실수있을거에요.

JunHoPark93 · 2022-03-02T15:10:20Z

src/main/java/controller/LottoController.java

-            }
-            return bonusNumber;
-        } catch (IllegalArgumentException e) {
+            return lottoService.calculateResult(inputView.getBonusNumber());


이거는 참고만 해보셔요!
메서드를 정의할때 보통 인풋과 아웃풋을 명확히 하는게 좋습니다.

결과계산 이라는 행위를 할때 인풋은 뭘까요?

내가산 로또, 당첨번호

가 필요하지 않을까요?

제이 말씀에 의하면, controller가 내가 산 로또, 당첨번호를 알고 있어야 -> service에게 이걸로 처리해주라고 부탁할 수 있습니다.
저는 controller가 아닌 service에게 도메인을 알고 시키는 것으로 코드를 작성해왔습니다. service만 도메인을 알고, 계속 필요하면 그것을 상태로 관리하게 하였습니다.

이부분에 대한 고민을 금일 상담전까지 고민해봐야겠네요 ㅠ!

그렇다면 제가 생각한 방법은

controller가 도메인이 아닌 원시값을 가지고 계산에 들어간다. (이렇게 하면 도메인 의존이 아니게 된다)

controller가 VO같은 것이 아니라면 Dto(혹은result클래스) 를 가지고 service에게 시킨다.

만일, 윙의 문제처럼 controller가 service에게 시킬 때, 내가산 로또, 당첨번호의 dto가 아닌 도메인이 필요하다면.. 가지고 있어도 되나..?

3번이 제일 고민입니다!

-> 일단 수정은 controller에서 도메인 편하게 사용하자. view에만 전달하지말자. 이번에는 result넘기는 것만 허용 정도로 정리해서 코드를 수정했습니다!

JunHoPark93 · 2022-03-02T15:13:51Z

src/main/java/dto/ResultDto.java

+    public EnumMap<RankPrize, Integer> getResultDto() {
+        return resultDto;
+    }


Result라는 도메인으로 바뀌게되면 dto반환부도 사라져야할텐데요,
일단은 dto는 신경쓰지말고 도메인도출, 역할분리 만 해주셔도 정말 충분할거같아요 👍
지금은 view에 도메인그대로 전달되도 좋습니다.

이 코멘트를 나중에 보고 고민하다가 시도한 내용은 아래와 같습니다!

geResultDto 및 Dto관련 내용삭제

outputView에서 dto를 받아 내부에서 통계관련 string처리 -> Result클래스에서 메소드에서 통계관련 string처리후 controller -> output까지 단일 String으로 결과 반환
outputView.printWinStatistics(result.getStatistics());

string처리는 view에서 해결되어야할 것 같은데 Result라는 결과(도메인)을 controller랑 view에 전달은 허용되었는데, string처리도 Result내부에서 하는게 맞는지 궁금합니다!

String처리를 분리하는게 좋습니다. 이 부분은 코멘트로 다시남겨둘게요!

JunHoPark93 · 2022-03-02T15:22:59Z

src/main/java/controller/LottoController.java

+    private void issueLotto() {
+        final int manualCount = getManualCount();
+        if (manualCount > MANUAL_LOTTO_IS_NONE_NUMBER) {
+            lottoService.issueLotto(getManualLottoWith(manualCount));
+            outputView.printLotto(lottoService.getIssuedLotto(), manualCount);
+            return;
+        }
+        lottoService.issueLotto(new ArrayList<>());
+        outputView.printLotto(lottoService.getIssuedLotto(), manualCount);
+    }


의문점 Q4. controller에서 inputview로 받는 수동 로또가 0개로 없는 경우, LottoService에 빈리스트를 넘겨주도록 분기하였는데 이게 맞는지 궁금합니다.

구조를 개선하는게 좋아보여요! 로또를 발급할때 수동개수 등 정보를 전달해주면 굳이 밖에서 분기를 해주지 않아도 발급된로또 getIssuedLotto()를 구할 수 있지 않을까요?

수동 개수를 전달해서 내부에서 처리하려고 했으나, 내부 try/catch 때문에 헤맸으나 early return + Collections.emptyList()를 이용해서 해결하였습니다 감사합니다 제이!

JunHoPark93 · 2022-03-02T15:32:10Z

src/main/java/service/LottoService.java

+    private final Money money;
+    private Lotto lastWinLotto;
+    private List<Lotto> issuedLotto;


불필요한 인스턴스변수인지 고민해보시면 좋을 것 같아요.
도메인 도출측면에서 생각해보면 lastWinLotto와 보너스번호를 묶어보면 당첨로또 라는 개념이 나올 수 있구요,
지난번에도 계속 이야기했던 부분이 역할분리인데요, 만들어주신 서비스가 비대해지는것을 막으려면 서비스에 도메인로직이 있는지 점검이 필요해요.

추가적으로 말씀드리면 추후 레벨에서 서비스를 굉장히 많이 다루실거에요. 지금은 서비스를 생각하지마시고 도메인로직으로 묶어볼수있는게 있는지 고민하시면 추후 미션들에 있어서 도움이 굉장히 많이 될거에요 👍

서비스가 비대해지는것을 막으려면 서비스에 도메인로직이 있는지 점검이 필요해요.

service에서 도메인에게 일을 시킬 때, 제가 인스턴스로 관리하는 도메인 및 중간 결과물들을

서비스가 비대할때는 도메인 추출을 생각

당첨로또 같은 새로운 도메인으로 추출 ->도메인 로직으로서 처리

하도록 빼야한다는 말씀이군요!
고민해서 적용해보도록 하겠습니다!

제가 컨트롤러에서는 도메인 사용안해야한다의 생각을 포니까..
서비스 상태 관리가 필요 없어졌습니다!

추가로 WinLotto의 클래스를 생성하여 관련 보너스볼 검증 로직 및 일부 로직이 도메인으로 넘어갔습니다.

말씀해주신 사항은 항상 되뇌이도록 잘 메모해야겠습니다 감사합니다 제이!

result라는 결과값 도메인을 controller에 반환가능 + 결과관련 로직을 담을 수 있게 허용하여 - service가 상태 관리하던 money -> controller에서 원시값으로 받아 여러번 사용 - Money에서 이익금을 받아 이익률 계산 로직을 Result에서 한번에 처리하도록 수정 - service에서 하던 이익률 계산을 -> 도메인 Result에게 위임

- controller에서 도메인 사용을 허용함 - LottoFactory는 Lotto만 생성하고 그 이후 로직은 service에게 책임 이전

is2js · 2022-03-03T08:17:12Z

2단계 수정사항 이라고 말씀드렸던 아래 부분

controller에서 도메인 의존성을 없애려고 노력했다.
- 이에 대한 것이 잘못되었는지 궁금합니다. service가 상태값으로 가지고 있지 않으려면, controller에서는 inputview에서 받은 데이터를 도메인사용하여 유지하고 있어야 했습니다! controller에서 Money, Lotto등을 사용하는게 정상적인 로직이 맞는지요?!
controller가 input/outputview의 객체를 사용하도록 노력했다.

내부 early return으로 바깥쪽 분기 제거

분기에 따라서... 특정함수의 반환값이 달라져야하는데, if early return + Collection의 빈값api 사용하도록 수정하였습니다.

기존: getManualLottoWith(manualCount) 내부에서 try/catch구문 때문에 빈값 return이 어려웠음.

private void issueLotto() {
    final int manualCount = getManualCount();
    if (manualCount > MANUAL_LOTTO_IS_NONE_NUMBER) {
        lottoService.issueLotto(getManualLottoWith(manualCount));
        outputView.printLotto(lottoService.getIssuedLotto(), manualCount);
        return;
    }
    lottoService.issueLotto(new ArrayList<>());
    outputView.printLotto(lottoService.getIssuedLotto(), manualCount);
}

수정: try/catch진입전에 응답type에 따른 빈값을 Collections api를 사용해서 제공

private List<Lotto> getManualLottoWith(final Count manualCount) {
    if (manualCount.isEnd()) {
        return Collections.emptyList();
    }
    try {
        final List<List<String>> manualLottoInput = inputView.getManualLotto(manualCount.getCount());
        return lottoService.issueManualLottoGroup(manualLottoInput);
    } catch (Exception e) {
        outputView.printInputError(e);
        return getManualLottoWith(manualCount);
    }
}


private void issueLotto() {
    final int manualCount = getManualCount();
    lottoService.issueLotto(getManualLottoWith(manualCount));
    outputView.printLotto(lottoService.getIssuedLotto(), manualCount);
}

service가 받은 후 여러번 사용되서 service에서 상태(인스턴스변수) 관리되었던 Money를 상태변수에서 제거 -> 대신 Controller에서 도메인사용 허용

외부에서 필요한 값을 받을 때마다 getMoney() 등으로 service에 넣은 후에야 객체 생성해야한다는 강박(도메인 의존성은 service에만, controller는 없어야한다)의 생각을 버리고 -> controller에서 도메인 생성/사용 허용함. 대신 View에 넘길 때만 DTO + 결과값클래스를 사용함. 으로 정리
- controller에서 바로 객체 생성시 -> controller가 의존성이 생겨 거부 -> input에서 받는 족족 service에 넣어서 초기화시켜주고 사용바로 해야만 했다. -> 이게 잘못된 생각이었던 것일까요?
  - controller에서 Money, Count, Lotto...등 도메인 객체 사용허용함..
  - **service는 상태관리 없이 시키기**만함.
- Q. 이게 맞을까요? 그렇다면.. service는 상태 값을 최소화하고, 도메인한테 시킨 결과값을 반환하고, controller는 도메인 결과값을 응답 받아도 되는 곳일까요??

LottoFactory는 로또 1장 발급만하는 정적펙토리메서드를 모은 것 -> LottoService에서 발급된 로또 모으게 하기

LottoFactory는 단순하게 복잡한 정적 팩토리 메서드 로직만 따로 빼서 class로 만든 것일까?
- 아니면.. Lotto 및 Lotto List까지, 관련된 모든 생성 로직들을 다 모아도 되는 것인가?
- AutoLotto와 ManualLotto가 비슷한 로직을 가지고 있으니, generator(구상체)마다 다른 것을 받아서 생성해주는 only Lotto 생성 만을 위한 클래스인가

service에서 최대한 시킨 결과물을 한꺼번에 다 뽑게 -> 못뽑으면 controller에서 중간결과물(도메인)을 받아, 추가로 service에게 시키기

outputView.print()에만 컨트롤러에서 쓰던 도메인을 dto로 변경해서 넘겨주기

필요에 의한 도메인 생성 WinLotto

1개만 생성될 것이고, 기본적으로 Lotto와 차이점을 못느꼈으나
1. Lotto와는 다르게 보너스볼과 중복되면 안되는 추가 검증이 필요하다
2. 보너스볼과 함께 움직인다. -> 묶어서 1개의 도메인으로 만든다.
  - 함께 움직이다 보니, 안 묶어주면 비교로직에서 인자가 기본 3개이상 되어린다.
```
final Lotto winLotto = getWinLotto();
final LottoNumber bonusNumber = getBonusNumber();
lottoService.calculateResult(issuedLotto, winLotto, bonusNumber);
```

Count를 그대로 쓰되, 수동Count 생성시 검증은 정적팩토리메서드에서 따로??

따로 수동 갯수 입력시 투입 금액에 따른 전체 갯수와 비교하는 검증이 추가로 필요했습니다.
- ManualCount 클래스를 만들기보다
- Count에서 생성자를 추가해서 구현했는데, 이렇게 구현해도 될지요?
  - **이게 가능하다면.. WinLotto도 Lotto로만 생성은 가능할 것 같은데, 대신 보너스볼과 같이 움직이면서 묶일 순 없겠네용.. 헷갈립닏다! **

public Count(final int count) {
    this.count = count;
}

public static Count createWithTotal(final int manualCount, final int totalCount) {
    validate(manualCount, totalCount);
    return new Count(manualCount);
}

private static void validate(final int manualCount, final int totalCount) {
    if (manualCount > totalCount) {
        throw new IllegalArgumentException(MANUAL_COUNT_CANNOT_EXCEED_TOTAL_MESSAGE);
    }
}

generator를 전략적으로 나눠받아야하는데, 어차피 2개가 다 쓰여서 외부입력을 안하고 service내부에서 LottoFactory에 바로 생성해서 전달하여 사용중인데 이게 맞나??

// service
  public List<Lotto> issueManualLottoGroup(final List<List<String>> issuedManualLottoInput) {
      final List<Lotto> issuedManualLotto = new ArrayList<>();
      for (List<String> manualLottoInput : issuedManualLottoInput) {
          issuedManualLotto.add(LottoFactory.generateLotto(new ManualLottoGenerator(manualLottoInput)));
      }
      return Collections.unmodifiableList(issuedManualLotto);
  }

  public static List<Lotto> generateAutoLottoGroup(final int count) {
      final List<Lotto> issuedAutoLotto = accumulateAutoLottoWithCount(new AutoLottoGenerator(), new Count(count));
      return Collections.unmodifiableList(issuedAutoLotto);
  }

JunHoPark93

안녕하세요 돌범!
2단계 피드백 반영하시느라 고생많으셨습니다.
관련해서 몇가지 추가로 요청드려요. 같이 이야기한 내용 토대로 한 두 부분 더 개선해보시면 도움이 많이 될 거에요 👍
시간은 충분히 남았으니 차분히 고민하는 시간으로 활용하셔도 좋을 것 같아요.

JunHoPark93 · 2022-03-03T11:59:03Z

src/main/java/domain/Result.java

+    private static final String SECOND_RANK_CORRECT_MESSAGE = "개 일치, 보너스 볼 일치(";
+    private static final String RANK_PRICE_MESSAGE = "원)- ";


말씀드린것처럼 도메인에서 뷰와 관련된 처리를 한번 분리해보면 좋을 것 같아요!
결과라는 측면을 해보고싶다고하셨던 TDD로 다시 만들어보시는건 어떨까요?

해당 로직을 view로 가져갔습니다 감사합니다 제이!

JunHoPark93 · 2022-03-03T12:00:32Z

src/main/java/controller/LottoController.java

-
-    public LottoController() {
-        lottoService = new LottoService(getMoney());
+    public LottoController(final InputView inputView, final OutputView outputView) {


컨트롤러도 도메인로직이 있을지 다시한번 둘러보시고 한눈에 읽히기 좋게 흐름을 만들어보면 좋을 것 같아요!

컨트롤러 부분에 input에 따른 -> output으로 드러나도록 메서드 호출 위치나 메서드 추출을 변경해보았습니다.

나름 한다고 했는데 불편하실 수 도 있을 것 같습니다! ㅋㅋ^^

제이 감사합니다! 제이를 만난 것은 행운이에요

프리코스에서부터 달고 왔던 궁금증이 다 풀렸습니다!!!!!!!!!

- InputView#getManualLotto의 returnType에서 List의 List 구조 제거 - Controller에서 service를 사용하는 로직의 흐름이 자연스럽게 input/output 구조를 변경함.

is2js · 2022-03-03T15:30:21Z

제이! 오늘 쪽찝게 피드백 덕분에 프리코스 때부터 갈증 났던 부분이 많이 해소 되었습니다.ㅠ

감사합니다!!! 적게 일하시고 많이 버시는 개발자 되십시오!!
알려주신 것 바탕으로 많이 채워보겠습니다!!

JunHoPark93

안녕하세요 돌범!
재차 요청드렸는데 마지막까지 반영하시느라 고생많으셨습니다 👍
이번 미션이 다음 미션 진행하시는데 도움이 되었으면 합니다.
이만 머지하겠습니다. 다음 미션도 화이팅이에요~

JunHoPark93 · 2022-03-04T09:20:46Z

src/main/java/view/OutputView.java

+        System.out.println(stringBuilder);
+    }
+
+    private String getStatisticsWith(final Entry<RankPrize, Integer> rankCount) {


잘 옮겨주셨네요 👍

is2js added 30 commits March 2, 2022 16:24

feat: 로또 번호 일급컬렉션 구현

b6fcb38

feat: 구입 금액만큼 로또 갯수로 변환하는 기능 구현

b431c39

feat: 로또 발급 횟수 Count 구현

1c1be05

feat: 자동 로또 발급 기능 구현

fb6ca8e

feat: 구입 금액만큼 로또를 발급하는 기능 구현

648ea1d

feat: 로또 발급시 오름차순 정렬 기능 구현

30268cc

feat: 발급 로또의 당첨 번호 일치 갯수 확인 기능 구현

8b65a4f

feat: 당첨 상태에 따른 수익률 계산 기능 구현

30f9619

지난 주 당첨번호 및 보너스 번호 입력받는 기능 구현

15d25c2

feat: 당첨 통계를 출력하는 기능 구현

40ee4e7

refactor: 매직넘버 상수화를 포함한 전반적인 리팩토링

8dc5243

refactor: 테스트용 메서드 삭제 및 접근제한자 수정

f495d4c

refactor(LottoFactory): 결과값 저장 인스턴스 변수를 제거하고 지역변수로 응답하도록 수정

e436f2d

refactor:(LottoFactory): 2개 로직 각 메소드로 추출하여 분리

5871406

style: 메서드명 변경

2353090

style: 클래스명 변경(->LottoService, WinCount->MatchedCount)

52c8f83

refactor: controller가 money 의존성을 끊도록 설계 수정

3c1082f

refactor: Controller의 Lotto 대신 LottoDto를 의존하도록 수정

51e60df

refactor: controller에서 Input,OutputView 객체를 파라미터로 받도록 수정

48f8ad2

refactor: RankPrice -> RankPrize 클래스명 변경 및 리팩토링

15fd2bf

- boolean field를 제거하고 filter 하나를 추가하여 SECOND, THIRD를 감별 - NONE 객체 추가

refactor: InputView, OutputView 메서드들 static 제거

13fdca3

refactor: LottoService가 중간결과물 issuedLotto를 인스턴스 변수로 관리하도록 수정

e86d758

fix: RankPrize 테스트 수정 및 메서드명 수정

393a424

feat(LottoNumber) cache를 통한 객체 생성 기능 구현

a78c759

style: 불필요한 코드 제거 및 warning 해결

314e2f1

feat: auto(random) Lotto 생성 로직을 클래스로 추출

fee1170

feat: autoLotto 생성 로직 변경(개별 랜덤 -> shuffle과 인덱싱)

7a06031

docs: 기능 구현 목록 수정(2단계 반영)

88d9a91

feat: Lotto generate후 collect 로직을 LottoFactory class로 추출

4d1deed

is2js added 8 commits March 2, 2022 16:25

feat: 입력한 수만큼 반복해서 수동 로또를 입력받는 기능 구현

517d319

- 만약 반복 횟수가 0이라면, IntStream.collect(Collectors.toList())는 [] 빈 List를 반환해준다.

fix: 수동 로또가 0개인 경우에 수동 로또 번호를 입력받는 버그를 수정

9c659b6

feat(LottoFactory): generator 주입 후 로또종류별 Lotto 1개 혹은 count만큼 LottoLis…

f19eef3

…t 발급 기능 구현

refactor: LottoService를 domain->service 패키지 이동

2533048

fix: early return에 의한 print 누락 부분 수정

285ddae

feat: 발급된 전체 로또 출력시 수동과 자동 개수 구분 메세지 추가

e17db41

style: 코드 컨벤션 적용

1462270

fix: 코드 중복 제거

3e3739b

JunHoPark93 requested changes Mar 2, 2022

View reviewed changes

is2js added 13 commits March 3, 2022 10:55

refactor(Controller): 중복되는 코드에 대한 로직 변경

4c513ce

refactor: 결과값을 책임지는 클래스명 및 위치 변경 ResultDto->Result

0d8f552

fix: conflict 미해결로 인한 코드 중복 제거

8ececd2

style: Lotto getter 메서드 명칭 변경(get->getLotto)

2fe256d

style: 컨벤션 미적용 메서드명 수정함(accumulateAutoLottoWithCount)

5b3598e

feat: 통계 출력부분을 Result 클래스가 담당하도록 수정

0800c9e

refactor: controller에서 Money(도메인) 사용 허용에 따른 메서드 구조 변경

439e996

refactor: service의 인스턴스 변수 제거에 따른 리팩토링

a079d79

- controller에서 도메인 사용을 허용함 - LottoFactory는 Lotto만 생성하고 그 이후 로직은 service에게 책임 이전

feat: 당첨번호와 보너스번호를 묶어 WinLotto 클래스 추출 적용

7054917

feat: 수동 로또 수 입력이 투입 금액을 넘어설 때 예외 발생시키는 기능 구현

3923e2d

feat: ManualCount 검증후 생성을 위한 정적 팩토리 메서드 및 테스트 구현

5d53551

style: 코드 컨벤션 적용

a744841

JunHoPark93 requested changes Mar 3, 2022

View reviewed changes

is2js added 3 commits March 3, 2022 23:37

test: Lotto#compare 테스트 코드 추가 구현

f0003c1

refactor: Result에 있던 string관련 로직 OutputView로 이동

6732ef2

refactor: 명시적인 input,output을 가지도록 구조 변경

3575088

- InputView#getManualLotto의 returnType에서 List의 List 구조 제거 - Controller에서 service를 사용하는 로직의 흐름이 자연스럽게 input/output 구조를 변경함.

JunHoPark93 approved these changes Mar 4, 2022

View reviewed changes

JunHoPark93 merged commit 7c6f9be into woowacourse:is2js Mar 4, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[2단계 - 로또(수동)] 돌범(조재성) 미션 제출합니다. #451

[2단계 - 로또(수동)] 돌범(조재성) 미션 제출합니다. #451

is2js commented Mar 2, 2022 •

edited

JunHoPark93 left a comment

JunHoPark93 Mar 2, 2022

is2js Mar 3, 2022

JunHoPark93 Mar 2, 2022

is2js Mar 3, 2022 •

edited

JunHoPark93 Mar 3, 2022

JunHoPark93 Mar 2, 2022

JunHoPark93 Mar 2, 2022

JunHoPark93 Mar 2, 2022

is2js Mar 3, 2022

JunHoPark93 Mar 2, 2022

is2js Mar 3, 2022

JunHoPark93 Mar 3, 2022

JunHoPark93 Mar 2, 2022

JunHoPark93 Mar 2, 2022

is2js Mar 3, 2022 •

edited

JunHoPark93 Mar 2, 2022

is2js Mar 3, 2022

JunHoPark93 Mar 3, 2022

JunHoPark93 Mar 2, 2022

is2js Mar 3, 2022

JunHoPark93 Mar 2, 2022

is2js Mar 3, 2022

is2js Mar 3, 2022

is2js commented Mar 3, 2022 •

edited

JunHoPark93 left a comment

JunHoPark93 Mar 3, 2022

is2js Mar 3, 2022

JunHoPark93 Mar 3, 2022

is2js Mar 3, 2022

is2js commented Mar 3, 2022

JunHoPark93 left a comment

JunHoPark93 Mar 4, 2022

		private static final String SECOND_RANK_CORRECT_MESSAGE = "개 일치, 보너스 볼 일치(";
		private static final String RANK_PRICE_MESSAGE = "원)- ";

[2단계 - 로또(수동)] 돌범(조재성) 미션 제출합니다. #451

[2단계 - 로또(수동)] 돌범(조재성) 미션 제출합니다. #451

Conversation

is2js commented Mar 2, 2022 • edited

수정사항

피드백 반영내역

JunHoPark93 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

is2js Mar 3, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

is2js Mar 3, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

is2js commented Mar 3, 2022 • edited

2단계 수정사항 이라고 말씀드렸던 아래 부분

내부 early return으로 바깥쪽 분기 제거

service가 받은 후 여러번 사용되서 service에서 상태(인스턴스변수) 관리되었던 Money를 상태변수에서 제거 -> 대신 Controller에서 도메인사용 허용

LottoFactory는 로또 1장 발급만하는 정적펙토리메서드를 모은 것 -> LottoService에서 발급된 로또 모으게 하기

service에서 최대한 시킨 결과물을 한꺼번에 다 뽑게 -> 못뽑으면 controller에서 중간결과물(도메인)을 받아, 추가로 service에게 시키기

필요에 의한 도메인 생성 WinLotto

Count를 그대로 쓰되, 수동Count 생성시 검증은 정적팩토리메서드에서 따로??

generator를 전략적으로 나눠받아야하는데, 어차피 2개가 다 쓰여서 외부입력을 안하고 service내부에서 LottoFactory에 바로 생성해서 전달하여 사용중인데 이게 맞나??

JunHoPark93 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

is2js commented Mar 3, 2022

JunHoPark93 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

is2js commented Mar 2, 2022 •

edited

is2js Mar 3, 2022 •

edited

is2js Mar 3, 2022 •

edited

is2js commented Mar 3, 2022 •

edited