[1단계 - 로또 구현] 검프(김태정) 미션 제출합니다. #269

Livenow14 · 2021-02-18T09:49:29Z

안넝하세요 코니!!!

우테코 3기 백엔드 과정 검프입니다. 만나서 반갑습니다 이번 미션 잘 부탁드립니다 😊

이번 미션을 진행하며, 원시값을 최대한 사용하지 않으려 했어요. 그러다 보니 로직이 더 복잡해지고, 어떤 객체에게 어떤 메시지를 보내야할지 정하는 것이 정말 어려웠어요.

이번에는 2가지를 신경써서 코드를 작성해봤어요.

첫 번째, 원시값 포장

public class LottoBall {
    private static final int MIN_LOTTO_VALUE = 1;
    private static final int MAX_LOTTO_VALUE = 45;

    private final int value;

    public LottoBall(final int value) {
        validateNumber(value);
        this.value = value;
    }

LottoBall 이라는 로또의 번호를 저장할 객체를 만들었고,

public class LottoBalls {
    private static final int LOTTO_NUMBERS_SIZE = 6;
    private static final int SPLIT_THRESHOLD = -1;
    private static final String DELIMITER = ", ";

    private final List<LottoBall> lottoBalls;

로또 리스트를 저장할 일급 컬렉션인 LottoBalls를 만들었어요

public class Lotto {
    private final LottoBalls lottoBalls;

Lotto는 LottoBalls를 가지고 있는게 어색하지 않아 보였고,

public class Lottos {
    private final List<Lotto> lottos;

Lottos는 Lotto리스트를 가지게 했어요.

여기서 질문.

Lottos에서 번호를 확인하려면 List → LottoBalls → LottoBall → value 이라는 엄청난 과정을 거쳐야 하더라고요.. get을 최대한 지양하고, 상태를 가져오지 않게 하려 했지만, 쉽지 않았어요.

여기서 LottoBalls는 괜히 만들었나? 라는 생각이 스멀 스멀 올라오더라고요.. 이에 대한 코니의 생각이 궁금해요!

두 번째, 다중 dto

위와같이 작성하다보니 Lotto에서 LottoBalls를 확인하려 dto를 작성했는데 Lotto의 인스턴스 변수인 LottoBalls를 바로 가져오는게 맞나 싶더라고요.

public class LottoResponseDto {
    private final LottoBallsDto lottoBallsDto;

    public LottoResponseDto(Lotto lotto) {
        this.lottoBallsDto = new LottoBallsDto(lotto.findLottoNumbers());
    }

    public LottoBallsDto getLottoNumbersDto() {
        return lottoBallsDto;
    }
}

그래서 위와같이 dto를 하나 더만들어서 dto에서 dto를 반환하게 했고

public class LottoBallsDto {
    private final List<LottoBall> lottoBalls;

    public LottoBallsDto(List<LottoBall> lottoBalls) {
        this.lottoBalls = lottoBalls;
    }

현재와 같이 작성했습니다.

여기서 질문.

LottoBallsDto의 인스턴수 변수를 List로 만들려다 현재와 같이 정의를 했습니다.

이렇게 dto에 dto가 연쇄사슬처럼 들어가게 되는 경우는 로직을 잘못짠 경우인가요??ㅠㅠ

아니면, 이렇게 dto에 dto를 포함시키는게 맞는 일인가요? 많은 피드백 바랍니다!

차근차근 하나씩.

애매했던 부분들이 많아서 코드 작성이 더뎠던거 같습니다.

코드를 잘 작성하는 개발자로 성장하고 싶습니다.

코니의 피드백을 통해 더 성장하겠습니다.

감사합니다 😊

구현할 기능을 README로 정리하였음

협력에 필요한 메세지를 먼저 정의 하였음.

이미지를 통해 객체 협력을 정리하였다

중복 검사, 번호 검사, 갯수 검사 완료

음수, 실수가 들어모면 에러가 발생한다.

보장된 당첨 번호(중복되지 않은 1~45의 숫자, 6개의 숫자) 보장되지 않은 당첨번호 에러가 발생한다.

로또 번호를 가질 LottoNumber 객체 생성 및 검증 완료

LottoNumber을 관리할 LottoNumbers 객체를 생성 및 검증하였음

private final List<Integer> lottoNumbers; -> private final LottoNumbers lottoNumbers;

BonusNumber 객체 생성 및 검증 완료

생성자를 private으로 감추고, 정적 팩토리 메서드를 생성하였음

구현 부분에서 구입금액을 입력 후 저장하였음

RandomLottoUtil 객체 생성

int value를 반환하는 Budget메소드를 추가하였음.

dto를 통해 로또 번호 출력

LottoGameMachine의 책임을 Screen에 옮김

Number -> ball 로 변경하였음

dto의 이름을 변경하였음

Livenow14 · 2021-02-18T10:09:22Z

1개의 요소를 갖는 리스트 만드는법 - 0.5

#java #jdk #list #Collections

배운것

java.util.Colletions애는 Colletions프레임워크 타입의 객체에 대한 객체생성, 정렬, 병합, 검색 등의 알고리즘을 구현한 메소드가 정의되어 있습니다.

저는 그중 Colltions.singletonList()를 사용해보았습니다.

기술설명

java.util.Colletions에는 아래와 같이 메소드가 정의되어있는데요

public static <T> List<T> singletonList(T o) {
        return new SingletonList<>(o);
    }

전달인자로 들어온 객체만 포함하는 불변의 리스트를 반환하는 메소드입니다.

예제

이 기술을 사용하기 전에는

Lottos lottos = new Lottos(Arrays.asList(
						new Lotto(LottoBalls.generate(lottoNumbers))));

위와 같이 사용했는데

이후엔

Lottos lottos = new Lottos(Collections.singletonList(
						new Lotto(LottoBalls.generate(lottoNumbers))));

와 같이 사용할 수 있었습니다.

장단점

장점
- 메모리를 조금더 아낄수 있다고 합니다.
- 제가생각하는 장점으로는 리스트의 불변을 보장하며 생성할 수 있다는 점입니다.
단점
- 사용할 곳이 많을까? 라는 생각이 듭니다.

choihz

안녕하세요 검프, 리뷰어 코니입니다 👋
우선 메세지 정의와 협력 정리는 정말 💯 그리고 코드를 읽으면서, 정말 많은 고민을 하셨겠구나 하는 생각이 들었어요. 다만 제가 아래에 코멘트로도 많이 남겼듯이, 지금은 구조도 그렇고 전반적으로 과하게 복잡하다는 느낌이 들어요. 저는 차라리 정적 팩터리 메서드와 DTO 같은 것은 모두 잊어버리시라고 말씀드리고 싶은 심정이에요 😭😭 그리고 남겨주신 질문에 답을 남기려고 보니, 코멘트만 읽어도 제가 뭐라고 답할지 짐작이 가능하실 것 같네요(…)

다만 저와 의견이 다른 부분도 충분히 (많이?) 있을 것 같은데요, 적극적으로 코멘트나 DM을 주시면 다양한 고민과 생각을 주고받을 수 있지 않을까 해요. 검프의 의견 기다릴게요! 😊

choihz · 2021-02-19T12:07:36Z

docs/README.md

+## 객체 협력 정리
+![객체 협력 정리](../image/객체협력정리.png)
+
+
+## 매세지 정의 


와우 👍

choihz · 2021-02-19T15:02:38Z

src/main/java/view/dto/LottoBallsDto.java

+
+import java.util.List;
+
+public class LottoBallsDto {


(아래는 제 의견일 뿐이니 가볍게 읽어 주세요. 검프의 의견도 궁금해요~)

이 어플리케이션에 DTO가 무려 5개나 있군요! 혹시 검프가 DTO를 만들어야겠다고 생각하신 이유는 뭘까요? 그리고 이 DTO를 만듦으로써 어떤 장점을 느끼셨나요? 혹은 단점은요?

제가 느끼기엔 로또 어플리케이션은 구조도 심플하고 도메인도 간단해서 DTO의 필요성이 크지 않아요. 오히려 DTO로 인해서 여기저기에 변환 로직이 추가되었고, 너무 많은 이름과 객체들이 지나치게 복잡한 코드를 만들고 있는 것 같아요.

MVC 모델이나 DTO의 개념이라든가, 우리가 지금 열심히 배우고 있는 클린 코드, 객체지향 등등의 궁극적인 목적은 무엇일까요? 궁극적인 목적이 좋은 코드라고 해볼까요? 그럼 좋은 코드란 무엇일까요? 코드가 사용되는 맥락에 따라, 그리고 각자의 생각에 따라 다를 수는 있지만 저는 결국엔 읽기 쉬운 코드가 좋더라고요. 읽기 쉬우면 파악하기 쉽고, 파악하기 쉬우면 기능을 확장하기도, 오류를 발견하고 수정하기도 쉬워요. 결국 이런 코드가 개발자의 시간을 많이 절약해 줍니다. 실제로 일하다 보면 코드를 읽는 시간이 정말 많거든요 😭

정성을 많이 들인 코드이기 때문에 한 줄 한 줄이 익숙하겠지만, 머리를 가볍게 비우고 로또 어플리케이션 개발팀에 새로 합류한 개발자의 입장에서 코드를 한 번 훑어보면 좋을 것 같아요. 코드를 이해하는 데 얼마나 걸릴지, 어떤 추가 요구사항이나 오류 수정 요청이 들어왔을 때 얼마나 쉽게 해결할 수 있을지도 생각해 보면 👍

이 어플리케이션에 DTO가 무려 5개나 있군요! 혹시 검프가 DTO를 만들어야겠다고 생각하신 이유는 뭘까요? 그리고 이 DTO를 만듦으로써 어떤 장점을 느끼셨나요? 혹은 단점은요?

제가 dto를 사용했던 이유는 도메인의 인스턴스를 그대로 view에 넘겨주지 않고, 필요한 값만 dto를 통해 view에 넘겨주기 위함이었어요..!

view가 domain의 상태값을 그대로 가지고 있는게 뭔가 어색해 보였거든요...😭

dto를 사용하면서 느꼈던 장점은 view의 요구사항이 변경되었을 때, dto만 변경해줘도 된다는 것을 알게 됐다는 점입니다.

단점은, 코니가 말씀하셨듯이 구조적으로 많은 로직들이 추가되어 이해하기가 힘든 코드가 되었다는 점인거 같습니다!

MVC 모델이나 DTO의 개념이라든가, 우리가 지금 열심히 배우고 있는 클린 코드, 객체지향 등등의 궁극적인 목적은 무엇일까요? 궁극적인 목적이 좋은 코드라고 해볼까요? 그럼 좋은 코드란 무엇일까요?

나말고 다른사람이 읽기 좋은 코드라 말할 수 있을 거 같습니다. 개념적으로 완벽해도 읽기 싫은 코드라면, 잘 짜여진 코드이지,좋은 코드는 아니라 생각합니다.

리팩토링
dto를 제거하였고, 객체들이 해야할 책임을 정리해보았습니다.
2fac4d6

분명히 DTO의 장점과 단점을 잘 알고 있으신 것으로 보여요 👍 다만 말씀드린 것처럼 이번 미션에서는 레이어의 경계도 모호하고, 도메인과 뷰가 알아야 할 정보 차이가 있는 것도 아니어서 단점이 많이 부각되었을 뿐인 거죠. 이번에 잘 사용해 보셨기 때문에, 그리고 그 필요성과 장단점을 충분히 고민해 보셨기 때문에 다음에 (곧!) 필요할 때는 정말 잘 사용하실 수 있을 거예요!

choihz · 2021-02-19T15:07:11Z

src/main/java/domain/LottoGameMachine.java

+    public Lottos makeLottos() {
+        LottoCount lottoCount = calculateLottoCount();
+        LottoGameScreen lottoGameScreen = new LottoGameScreen();
+        lottoGameScreen.showLottoCount(new LottoCountResponseDto(lottoCount.getLottoCount()));
+
+        Lottos lottos = makeLottos(lottoCount);
+        List<Lotto> lottoGroup = lottos.getLottos();
+
+        List<LottoResponseDto> lottoResponseDtos = makeLottoResponseDtos(lottoGroup);
+        lottoGameScreen.showAllLottoStatus(lottoResponseDtos);
+        return lottos;
+    }


이 메서드가 로또를 만드는 일만 하고 있나요?

이 부분을 보며 현재 많은 메서드들이 한가지 이상의 책임을 하고있다는 것을 알게되었습니다. (그래서인지 한 커밋에 너무 많은 변경이 들어갔어요..ㅠㅠ)
makeLottos는 아래와 같이 변경하였습니다.

public List<LottoTicket> buyTickets(BettingMoney bettingMoney) { int ticketCount = bettingMoney.getTicketCount(); return IntStream.range(0, ticketCount) .mapToObj(count -> makeTicket()) .collect(Collectors.toList()); } private LottoTicket makeTicket() { List<LottoBall> lottoBalls = LottoBalls.getRandomLottoBalls(); return new LottoTicket(new LottoBalls(lottoBalls)); }

2fac4d6

choihz · 2021-02-19T15:12:20Z

src/main/java/util/InputUtil.java

+
+import java.util.Scanner;
+
+public class InputUtil {


InputUtil과 OutputUtil은 굳이 유틸로 분리될 필요가 있을까요? 분리했을 때의 장점이 무엇이라고 생각하시나요?

유틸로 분리한 이유는,
현재 요구사항인 "콘솔에서 출력"을 구현함에 있어 InputUtil와 OutputUtil이 Static화 되여 생명주기가 프로그램과 함께해도 괜찮을 것이라 생각했기 때문이에요.

제가 생각하는 이렇게 사용했을 때 장점은

클래스의 입력과 출력을 요구하는 어느곳이든지 사용할 수 있다고 생각해요. 리팩토링한 클래서에서는 입, 출력이 controller에서만 진행되네요...😂 (인스턴스화를 통해 객체를 만들어야할까요..?ㅎㅎ)

단점은

프로그램 생명주기를 함께함으로 생기는 메모리누수, 쓰레드 세이프하지 않을 문제가 있을거 같고, 애플리케이션의 협력을 봤을 때는 협력이 정확히 그려지지 않는다는 점 같아요..

장점과 단점을 쓰다보니, 단점이 더 큰거같아요.. 코니의 생각은 어떠신지 알 수 있을까요?

제가 여기서 위와 같은 질문을 드렸던 이유는, view 패키지 안에 있는 클래스들과 합쳐저도 충분할 것 같다는 생각이 들어서였어요 ㅎㅎ 유틸과 이 패키지 안의 클래스들이 이미 굉장히 강하게 결합하고 있지 않나요?

choihz · 2021-02-19T15:14:02Z

src/main/java/domain/ball/BonusBall.java

+public class BonusBall {
+    private final LottoBall lottoBall;


이 객체도 분리될 만큼의 책임을 가지고 있지는 않은 것 같아요.

분리될 만큼의 책임임에 대해 진지하게 생각해보는 피드백이었어요!!
객체가 단지 어떠한 값을 보관하기 위한 용도라면, 분리되는 것이 의미가 없다는 것을 알게되었어요.
객체의 생성책임이 저장만 있다는 것도 이상하다는 것을 알게되었습니다 ㅎㅎ

BonusBall 객체를 삭제하였고, 이후 다른 클래스의 책임, 네이밍을 변경하엿습니다.

405a7de
7fb9ef8

choihz · 2021-02-19T15:54:41Z

src/test/java/domain/ball/LottoBallTest.java

+                .doesNotThrowAnyException();
+    }
+
+    @DisplayName("보장되지 않은 LottoNumber 에러 발생한다. ")


"보장되지 않은" 숫자가 무엇인지 더 자세히 알려주면 좋을 것 같아요.

"보장되지 않음"이라는 말이 중복될 우려가있어, docs/README.md에 보장되지 않음을 따로 명시해 두었는데,
이런 경우에도 테스트에 따로 명시를 해주는게 가독성 측면에서 더 좋다는 말씀이죠?!

@DisplayName("보장되지 않은(중복되지 않은 1~45의 숫자이며, 6개의 숫자) LottoBall이 저장될 시 에러 발생한다. ") @ParameterizedTest @ValueSource(ints = {-1, 0, 46, 47}) void lottoTicketNotGuaranteedErrorTest(int value) { Assertions.assertThatThrownBy(() -> new LottoBall(value)) .isInstanceOf(IllegalArgumentException.class); }

위와 같이 변경하였습니다.
c5bcd38

문서란 게 낡기 마련이라(계속 업데이트하기가 정말 힘들어요 😭), 웬만하면 코드와 테스트 코드에 항상 명시적으로 표현해 주는 것이 좋습니다.

네 항상 명시적으로 작성하는 습관을 들여야겠군요..! 확인했습니다..!! ㅎㅎ

choihz · 2021-02-19T16:02:59Z

src/main/java/domain/result/LottoRank.java

+    FIVE_AND_BONUS_MATCHES(5, Budget.amounts(30_000_000), true),
+    SIX_MATCHES(6, Budget.amounts(2_000_000_000), false);
+
+    private static final String STRING_FORMATTER = "%s개 일치 (%d원)";


사용되지 않고 있네요.

사용되지 않는값 수정하였습니다! b57f31f

choihz · 2021-02-19T16:03:16Z

src/main/java/domain/result/LottoRank.java

+
+    private final int matches;
+    private final Budget budget;
+    private final boolean isBonus;


이 값도 역시 사용되지 않고 있어요. 여기에 정의되어야 할 이유가 있을까요?

정의되어 있으면 좋을거 같아서 추가한 것인데, "나중에 필요하겠지"라는 생각으로 추가했던거 같습니다. 아래와 같이 변경하였습니다.

private final int matches; private final int prize; LottoRank(final int matches, final int prize) { this.matches = matches; this.prize = prize; }

b57f31f

출력 로직 변경으로 인해 다시 사용하게되었습니다.
b7fa142

choihz · 2021-02-19T16:03:36Z

src/main/java/domain/result/LottoRank.java

+        this.isBonus = isBonus;
+    }
+
+    public static LottoRank findByBonusWithMatches(boolean containBonus, int matches) {


이름이 메서드의 역할을 잘 드러내고 있나요?

이름이 메서드의 역할을 잘 수행하고 있지 않음을 확인햇습니다.

public static LottoRank findRankByBonusAndMatches(final boolean isBonus, final int matches) { if (!isBonus || matches != 5) { return Arrays.stream(values()).filter(lottoRank -> lottoRank.isSameMatches(matches)) .findFirst().orElse(NONE_MATCHES); } return FIVE_AND_BONUS_MATCHES; }

위와 같이 변경 하였습니다. b57f31f
혹시 현재와 같은 네이밍도 안좋은 네이밍일까요..?!

choihz · 2021-02-19T16:03:51Z

src/main/java/domain/result/LottoRank.java

+        return FIVE_AND_BONUS_MATCHES;
+    }
+
+    public int getMatches() {


역시 사용되지 않고 있군요 😞

사용되지 않는 값을 제거하였습니다. b57f31f

분리될 만큼의 책임을 가지지 않기 때문에 제거함.

- [x] LottoBalls 생성자 책임 정의 - > LottoBalls 생성자 제거 - [x] 입력 파싱 책임 제거 - [x] gnerate 제거 - [x] of 제거 - [x] winngLotto 생성 로직 변경

Lotto -> LottoTicket Lottos -> LottoTickets LottoCount -> TicketCount

Budget의 책임이 모호하여 BettingMoney로 변경함.

LottoService를 추가하여 LottoGameMachine의 책임을 이동하였고, DTO를 제거하면서 필요한 로직을 수정함. result 클래스 변경 및 테스트를 추가하였음

- [x] 사용되지 않는 값 제거 - [x] 메서드 네이밍 변경

출력 로직 변경 및 변경으로 인한 LottoRank 변경

모든 테스트의 네이밍을 변경하였음

의미없는 띄어쓰기 삭제 맟, 메소드 선언 순서 변경

리팩토링한 목록을 정리하였음

Livenow14 · 2021-02-22T03:10:49Z

choihz

안녕하세요 검프!

리뷰를 반영하는 게 정말 쉽지 않았으리라 생각하는데, 아주 잘해 주셨어요 👍 이번에 남긴 코멘트들은 참고하시고 다음 단계에서 반영해 주시면 됩니다. 다만, 제 의견과 다른 의견을 가지고 있으신 부분이 분명히 있을 거예요. 그런 부분에서는 적극적으로 본인의 생각을 표현해 주시면 제게도 큰 배움이 될 것 같습니다. 정말 고생 많으셨고, 2단계에서 만나요! 💯

choihz · 2021-02-22T03:06:47Z

src/main/java/GameManageApplication.java

+    public void run() {
+        BettingMoney bettingMoney = getBettingMoney();
+        TicketCount ticketCount = getTicketCount(bettingMoney);
+        mainScreen.showTicketCount(ticketCount);
+        LottoTickets lottoTickets = lottoService.getLottoTickets(bettingMoney);
+        lottoGameScreen.showAllLottoStatus(lottoTickets.getLottoTickets());
+        WinningLotto winningLotto = getWinningLotto();
+
+        Result result = new Result(lottoTickets, winningLotto);
+        lottoGameScreen.showGameResult(new LottoGameResultDto(result.getResults()));
+        lottoGameScreen.showRevenueResult(result.findEarningsRate(bettingMoney));
+    }


여기에서 이용하는 순서대로 아래에 메서드를 배치한다면, 위에서부터 읽어내려가기 편리할 것 같아요.

네 확인 후 리팩토링 진행했습니다.
d534d61

choihz · 2021-02-22T03:34:05Z

src/main/java/view/MainScreen.java

+public class MainScreen {
+    public static final String BUY_STATUS = "%d개를 구매했습니다.";
+
+    public void showInputMoney() {
+        OutputUtil.printMessage("구입금액을 입력해 주세요.");
+    }


이 클래스가 별도로 분리될 만큼의 책임을 가지고 있는지 고민해 볼까요? 보통은 작게 나누는 것이 좋긴 하지만, 때로는 연관된 책임이 나뉘어 있으면 오히려 탐색 비용이 올라가기도 하고, 응집도가 떨어져 한 가지의 수정을 여러 곳에서 해야 되기도 해요.

"구입금액을 입력해 주세요." 메시지는 사실 input을 위한 메시지니까 input view 쪽에서 출력해도 된다고 생각해요. 지금은 이렇게 분리되어 있어 컨트롤러에서 "메시지 먼저 출력해줘" → "구입 금액 받아와줘" 하며 일일이 지시하고 있네요.

input을 위한 메시지는 input에
출력을 위한 메시지는 output에
책임을 변경했습니다.
562e9c7

choihz · 2021-02-22T03:44:07Z

src/main/java/domain/LottoGameMachine.java

+    private LottoTicket makeTicket() {
+        List<LottoBall> lottoBalls = LottoBalls.getRandomLottoBalls();
+        return new LottoTicket(new LottoBalls(lottoBalls));
+    }


지금은 로또 생성을 위해 LottoBalls의 static 메서드인 getRandomLottoBalls()를 부르고, 이 결과를 다시 LottoBalls 생성자에 보내고 있는데 이 흐름이 조금 어색하게 느껴지네요. List<LottoBall>을 생성하는 책임은 LottoBalls 밖에 위치하는 게 더 적절해 보이고, 위치는 여기쯤이면 괜찮지 않을까 해요.

메서드들을 보면 LottoGameMachine보다 LottoMachine이 더 적절한 이름일 것 같아요. 이 클래스는 게임을 진행하는 것이 아니라, 돈을 받아 로또를 건네주는 역할을 하고 있으니까요.

물론 제 의견은 참고만 하시고요, 검프의 고민을 바탕으로 더 좋은 생각이 나온다면 훨씬 좋을 것 같아요!

책임을 어디에서 가지고 있는지 생각해볼 필요가 있다는 말씀이시죠?

LottoMachin으로 이름을 명명 하였고, 책임을 이동했습니다.
ddb0c9b

choihz · 2021-02-22T03:46:08Z

src/main/java/domain/ball/LottoBall.java

+public class LottoBall implements Comparable<LottoBall> {
+    public static final int MIN_LOTTO_VALUE = 1;
+    public static final int MAX_LOTTO_VALUE = 45;
+    public static final String PERMIT_LOTTO_NUMBER_EXCEPTION_MESSAGE = "%d~%d 사이의 번호만 허용합니다.";


LottoBalls에서는 예외 메시지를 따로 상수로 빼지 않았는데 여기서는 빼셨군요. 그렇다면 질문. 검프가 느끼기엔 어느 쪽이 더 편한 것 같나요? 더 편한 방법을 생각해 보고 그걸로 통일하면 더 좋을 것 같아요!

String.format을 이용하는 것은 최대한 상수로 빼고, 자주 쓰이지 않는 에러는 단순 출력이 좋다고 생각합니다.
이번 코드 작성에선 다 상수로 빼 보았습니다.
cc77469

choihz · 2021-02-22T03:50:14Z

src/main/java/domain/result/Result.java

+        int prize = results.entrySet().stream()
+                .map(Map.Entry::getKey)
+                .mapToInt(lottoRank -> lottoRank.getPrize() * results.get(lottoRank))
+                .sum();


Suggested change

int prize = results.entrySet().stream()

.map(Map.Entry::getKey)

.mapToInt(lottoRank -> lottoRank.getPrize() * results.get(lottoRank))

.sum();

int prize = results.keySet().stream()

.mapToInt(lottoRank -> lottoRank.getPrize() * results.get(lottoRank))

.sum();

확인했습니다 :)
e865c21

choihz · 2021-02-22T04:19:37Z

src/main/java/view/LottoGameScreen.java

+        lottoTickets.stream()
+                .forEach(lottoTicket -> showTicketStatus(lottoTicket));


Suggested change

lottoTickets.stream()

.forEach(lottoTicket -> showTicketStatus(lottoTicket));

lottoTickets.forEach(this::showTicketStatus);

Stream의 foreach 와 for-loop 는 다르다
위 글을 추천드려요! 😉

Stream.forEach()는 최종연산이기 때문에 동시성을 보장받기 힘들고 쓰레드에 안전하지 않습니다. 또한. 말 그대로 최종연산이기 때문에 로직이 추가되는것은 이상하다 볼 수 있습니다.
Coleections.forEach는 Stream객체를 새로 만들고 버리는 과정이 생략되기 때문에 성능향상을 이끌어 낼 수 있고, 또한 내부적으로 synchronized키워드가 있기 때문에 쓰레드 세이프합니다.

코니의 피드백 덕분에 애매했던 지식을 더 잘 알아가게 되었습니다. 감사합니다.
4487302

choihz · 2021-02-22T04:24:27Z

src/main/java/view/LottoGameScreen.java

+        List<String> status = lottoBalls.stream()
+                .map(lottoBall -> String.valueOf(lottoBall.getValue()))
+                .collect(Collectors.toList());
+        return LOTTO_PREFIX + String.join(DELIMITER, status) + LOTTO_POSTFIX;


Suggested change

List<String> status = lottoBalls.stream()

.map(lottoBall -> String.valueOf(lottoBall.getValue()))

.collect(Collectors.toList());

return LOTTO_PREFIX + String.join(DELIMITER, status) + LOTTO_POSTFIX;

return lottoBalls.stream()

.map(lottoBall -> String.valueOf(lottoBall.getValue()))

.collect(Collectors.joining(DELIMITER, LOTTO_PREFIX, LOTTO_POSTFIX));

와...! Collectors.joining이라는 멋진 기능이 있었네요!!
나중에 Collectors의 기능을 따로 정리할게요!! 감사합니다 😊

choihz · 2021-02-22T04:27:40Z

src/main/java/domain/result/Result.java

+    private final LottoTickets lottoTickets;
+    private final WinningLotto winningLotto;


이 두 인스턴스 변수는 Result가 가지고 있을 필요가 없어 보여요~ 생성할 때 받아서 Map<LottoRank, Integer>을 만들고 나면 필요하지 않으니까요.

생성시에만 필요한 인스턴스 변수는 확실히 따로 멤버변수에 둘 필요가 없네요!
좋은 지적 감사합니다. (현재 의미없게 쓰이는 곳도 더 살펴봐야겠어요 😊)

18a1327

choihz · 2021-02-22T04:28:15Z

src/main/java/domain/result/Result.java

+        return results;
+    }
+
+    private Map<LottoRank, Integer> setResult() {


리턴값이 사용되지 않고 있으니 반환하지 않아도 될 것 같아요.

반환이 필요하지 않는데 억지로 끼워 맞춘거 같네요 ㅎㅎ 좋은지적 감사합니다!!
다른 메소드들도 충분히 고민해보겠ㅅ브니다.
18a1327

choihz · 2021-02-22T04:36:24Z

src/main/java/util/InputUtil.java

+
+import java.util.Scanner;
+
+public class InputUtil {


제가 여기서 위와 같은 질문을 드렸던 이유는, view 패키지 안에 있는 클래스들과 합쳐저도 충분할 것 같다는 생각이 들어서였어요 ㅎㅎ 유틸과 이 패키지 안의 클래스들이 이미 굉장히 강하게 결합하고 있지 않나요?

pobiconan and others added 28 commits February 16, 2021 15:17

refactor: remove spark library, html

c24969d

docs: 기능 요구사항 정리

b2ac13f

구현할 기능을 README로 정리하였음

docs: 객체 메세지 정의

1a67798

협력에 필요한 메세지를 먼저 정의 하였음.

docs: 객체 협력 정리

3641f9c

이미지를 통해 객체 협력을 정리하였다

feat: 보장된 로또 번호로 한 로또를 생성한다

3d7f9e8

중복 검사, 번호 검사, 갯수 검사 완료

feat: 보장된 구입금액을 입력 받는다.

d7cd367

음수, 실수가 들어모면 에러가 발생한다.

feat: 구입금액 따른 로또의 갯수를 구한다.

6a4ee38

feat: 보장된 당첨 번호를 입력한다.

c27a39d

보장된 당첨 번호(중복되지 않은 1~45의 숫자, 6개의 숫자) 보장되지 않은 당첨번호 에러가 발생한다.

feat: LottoNumber 객체 생성

fbbc491

로또 번호를 가질 LottoNumber 객체 생성 및 검증 완료

feat: LottoNumbers 객체 생성

5414960

LottoNumber을 관리할 LottoNumbers 객체를 생성 및 검증하였음

refactor: Lotto 및 WinningLotto의 인스턴스 변수 변경

5f49491

private final List<Integer> lottoNumbers; -> private final LottoNumbers lottoNumbers;

feat: 보장된 보너스 볼을 입력 받는다

b0fed8e

BonusNumber 객체 생성 및 검증 완료

refactor: BonusNumber 정적 팩토리 메서드 생성

6c71cf3

생성자를 private으로 감추고, 정적 팩토리 메서드를 생성하였음

feat: 구현 부분 구입금액 입력 확인

6e4dbb8

구현 부분에서 구입금액을 입력 후 저장하였음

feat: 랜덤 로또 번호 생성

f5246b6

RandomLottoUtil 객체 생성

feat: 구입금액 입력 화면 출력 로직 생성

16fa84c

refactor: Budget 메소드 추가

f146ddc

int value를 반환하는 Budget메소드를 추가하였음.

feat: 구매 갯수 출력 로직 작성

28b49eb

feat: 로또 갯수만큼 보장된 로또번호를 출력한다.

8ef113f

dto를 통해 로또 번호 출력

refactor: LottoGameMachine::gameStart() 내부 구현 변경

f063afa

LottoGameMachine의 책임을 Screen에 옮김

feat: 임시 저장

84a2290

feat: 콘솔 창에 당첨번호를 입력한다.

457e440

feat: 로또 당첨 확인 로직 구현

0f30494

feat: 보너스 볼 포함 당첨 현황을 구한다

2d720cb

feat: 당첨 통계 출력

ad8cfda

feat: 수익률 계산 로직

48cce77

refactor: naming 변경

8a6a686

Number -> ball 로 변경하였음

refactor: dto naming 변경

f38d65b

dto의 이름을 변경하였음

choihz suggested changes Feb 19, 2021

View reviewed changes

Livenow14 added 11 commits February 21, 2021 13:58

refactor: BonusBall 객체 제거

405a7de

분리될 만큼의 책임을 가지지 않기 때문에 제거함.

refactor: WinningLotto, LottoBalls 책임 변경

cf1aa71

- [x] LottoBalls 생성자 책임 정의 - > LottoBalls 생성자 제거 - [x] 입력 파싱 책임 제거 - [x] gnerate 제거 - [x] of 제거 - [x] winngLotto 생성 로직 변경

refactor: 클래스 네이밍 변경

7fb9ef8

Lotto -> LottoTicket Lottos -> LottoTickets LottoCount -> TicketCount

refactor: Budget 클래스 BettingMoney로 변경

89799fc

Budget의 책임이 모호하여 BettingMoney로 변경함.

refactor: DTO 제거 및 책임 분리

2fac4d6

LottoService를 추가하여 LottoGameMachine의 책임을 이동하였고, DTO를 제거하면서 필요한 로직을 수정함. result 클래스 변경 및 테스트를 추가하였음

refactor: LottoRank 클래스 수정

b57f31f

- [x] 사용되지 않는 값 제거 - [x] 메서드 네이밍 변경

refactor: 에러 출력메세지 상수 활용

5b19a82

refactor: LottoGameScreen 변경

b7fa142

출력 로직 변경 및 변경으로 인한 LottoRank 변경

refactor: test 한글 네이밍 영어로 변경

c5bcd38

모든 테스트의 네이밍을 변경하였음

refactor: code convention

a487cfb

의미없는 띄어쓰기 삭제 맟, 메소드 선언 순서 변경

docs: 리팩토링 목록 정리

ee483cc

리팩토링한 목록을 정리하였음

choihz approved these changes Feb 22, 2021

View reviewed changes

choihz merged commit 36b0112 into woowacourse:livenow14 Feb 22, 2021

		lottoTickets.stream()
		.forEach(lottoTicket -> showTicketStatus(lottoTicket));

	lottoTickets.stream()
	.forEach(lottoTicket -> showTicketStatus(lottoTicket));
	lottoTickets.forEach(this::showTicketStatus);

		private final LottoTickets lottoTickets;
		private final WinningLotto winningLotto;

[1단계 - 로또 구현] 검프(김태정) 미션 제출합니다. #269

[1단계 - 로또 구현] 검프(김태정) 미션 제출합니다. #269

Conversation

Livenow14 commented Feb 18, 2021 • edited Loading

첫 번째, 원시값 포장

두 번째, 다중 dto

Livenow14 commented Feb 18, 2021 • edited Loading

1개의 요소를 갖는 리스트 만드는법 - 0.5

배운것

기술설명

예제

장단점

choihz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Livenow14 Feb 21, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Livenow14 Feb 21, 2021 • edited Loading

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

Livenow14 Feb 24, 2021 • edited Loading

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

Livenow14 Feb 21, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Livenow14 commented Feb 22, 2021 • edited Loading

리팩토링 목록

결론적으로

choihz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

choihz Feb 22, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Livenow14 Feb 23, 2021 • edited Loading

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

Livenow14 Feb 23, 2021 • edited Loading

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

Livenow14 commented Feb 18, 2021 •

edited

Loading

Livenow14 commented Feb 18, 2021 •

edited

Loading

Livenow14 Feb 21, 2021 •

edited

Loading

Livenow14 Feb 21, 2021 •

edited

Loading

Livenow14 Feb 24, 2021 •

edited

Loading

Livenow14 Feb 21, 2021 •

edited

Loading

Livenow14 commented Feb 22, 2021 •

edited

Loading

choihz Feb 22, 2021 •

edited

Loading

Livenow14 Feb 23, 2021 •

edited

Loading

Livenow14 Feb 23, 2021 •

edited

Loading