Step2 로또 게임 구현 완료했습니다. by imbf · Pull Request #1 · sproutt/heroes-java-lotto

imbf · 2020-03-09T02:05:21Z

일단 커밋을 제대로 남기지 못한점 죄송합니다!!!!

TDD로 처음에는 개발하다가 나중에는 그냥 production code부터 작성한것 같아요..

존재하는 모든 클래스에 Test 코드를 구현했습니다.
Test 코드와 함께 개발하다 보니까 흠.. 무언가 이상적인 코드에 가까워진 느낌?? (코알못의 생각입니다.)

어쨋든 이번 과제는 너무 힘들고 너무 재밌고 너무 많이 배웠네요.

추상화 공부좀 더 해야겠어요

imbf · 2020-03-09T06:12:06Z

+import java.util.List;
+import java.util.stream.Collectors;
+
+public class AutoLottoGenerator {


처음에는 RandomNumberGenerator를 사용해서 RandomNumber를 만든 후 AutoLotto로 변환하려고 했지만
굳이 돌아갈 필요 없이 AutoLottoGenerator에서 AutoLotto를 만들어도 나중에 Test할때 상관이 없겠다 싶어서 AutoLottoGenerator클래스를 만들었습니다.

imbf · 2020-03-09T06:12:52Z

+
+    private List<Integer> integers;
+
+    public AutoLottoGenerator() {


1~45까지의 정수가 저장된 List를 초기화 시키기 위한 생성자 입니다.

imbf · 2020-03-09T06:14:27Z

+        }
+    }
+
+    public List<Lotto> createLottos(int count) {


AutoLotto를 생성하기 위한 메소드 입니다. 나중에 Mock Object로 활용할 메소드 이기 때문에 정적 메소드로 만들지 않았습니다.

Mocking 을 하지 않아도 정적 메소드로 만들 이유가 별로 없어보이지 않나요?

imbf · 2020-03-09T06:20:16Z

+
+    private List<LottoNumber> lottoNumbers;
+
+    public Lotto(List<LottoNumber> lottoNumbers) {


원하지 않는 LottoNumbers가 들어오면 예외를 발생시켜서 객체 자체가 만들어지지도 않게끔 Lotto클래스의 생성자에 유효성검사 메소드를 두었습니다.
또한 객체 생성시 동시에 lottoNumbers가 정렬이 될 수 있도록 정렬 메소드도 생성자에 두었습니다.
정렬 메소드 관련해서 어디에 위치시킬까 많은 생각을 해보았지만 Lotto클래스의 생성자자체에서 LottoNumbers에 관한 정렬 책임을 가지는게 가장 이상적이다고 생각되어서 정렬 메소드도 생성자에 위치하였습니다.
나중에 다른 요구사항이 들어와도 lottoNumbers는 중복되지 않고 정렬도 필요하다고 생각해서 생성자에 많은 로직은 좋지 않지만 그럼에도 위치시켰습니다.

imbf · 2020-03-09T06:23:12Z

+
+public class LottoConverter {
+
+    public static List<Integer> convertLottoToIntegers(Lotto lotto) {


객체는 유일무이하기 때문에 Collection의 Contains 메소드를 활용할 수 없습니다.
그래서 어떻게 하면 Lotto객체를 WinningLotto객체와 비교할 수 있을 까 많은 고민을 해보았습니다... 그런 결과
원시데이터 형태로 바꾸면 비교할 수 있겠다 싶어서 Lotto 객체의 LottoNumbers를 List로 바꾸어 Contains 메소드를 사용할 수 있게끔 도와주는 convert 메소드를 구현하였습니다.

비교하기 위해서 원시데이터로 바꿔주는 작업이 비효율적인 것 같은데 다른 방법은 없을까요?

imbf · 2020-03-09T06:27:42Z

+    private PurchaseMoney purchaseMoney;
+    private int manualLottoCount;
+    private List<Lotto> lottos;
+    private WinningLotto winningLotto;


클래스 내의 인스턴스 변수의 갯수에 관해서는 정말 고민을 많이 했습니다!!
너무 많지 않나..? 라고 생각이 들었지만 줄이고 줄이고 줄여서 우리가 만들 수 있는 객체들은 LottoGame클래스에 객체변수로써 두지 않고 처리해서 반환값으로 다 바꾸어 버렸습니다.
하지만 단 하나 manualLottoCount 변수에 관해서는 없어도 된다고 생각 했는데 저희 요구사항에서는 없애도 되지만 조금만 요구사항이 바뀌면 로직이 정말로 복잡해지거나 다시 manualLottoCount 객체 변수를 만들 것 같아서 그냥 클래스 내의 객체 변수로 두었습니다.

imbf · 2020-03-09T06:30:05Z

+        return purchaseMoney.getMoney() / LOTTO_PRICE - manualLottoCount;
+    }
+
+    public void setLottos(List<Lotto> lottos) {


이 메소드 또한 정말 많이 고민한 메소드 입니다!!
우리가 어차피 클래스 내부에서 AutoLotto나 ManualLotto를 만들지 않는데 굳이 AutoLotto를 받을 메소드와 ManualLotto를 받을 메소드를 구분해야할까??? 라고 생각을 했지만 굳이..? 메소드 인자에서 티가 나는데? 라고 결론을 얻어서 AutoLotto를 받을 메소드와 ManualLotto를 받을 메소드를 합쳐서 그냥 setLottos 메소드 하나로 만들었습니다.

imbf · 2020-03-09T06:40:25Z

+        this.winningLotto = winningLotto;
+    }
+
+    public LottoResult createResult() {


제가 이번 프로젝트에서 가장 고민을 많이 한 메소드입니다!!!
RacingGame 클래스에서 도대체 게임 결과에 대해서 어디까지 책임을 맡아야 하는가?
단지 랭크의 결과를 리스트로 나열하면 되는건가? 그렇다면 개수는 어떻게 랭크별로 셀 수 있는가?
그렇다면 Map 인터페이스르 구현하는 HashMap<Rank, Count>을 사용해 볼까? 근데 안의 Key는 도대체 어떻게 바꾸지 그러면 로직이 너무너무너무 복잡해지는데? 정말 하루종일 A부터 Z까지 다 고민했습니다!!! 그러자 갑자기 유레카하면서 아 !! 그러면 View에 Result 값을 전달해주는 객체를 만들면 되겠다!! 그러면 Rank별로 Count를 저장할 수도 있고 수익률도 생성할 수 있겠구나 생각을 해서 LottoResult라는 클래스를 만들게 되었습니다!! 근데 어찌보니까 이게 Layer간의 데이터를 전달해주는 DTO(Data Transfer Object)와도 비슷한건가 궁금합니다. 어쨋든 제가 이번 프로젝트에서 생각 한 것 중에서 가장 맘에 드는 메소드 입니다!!

imbf

LottoGame을 구현하면서 들었던 많은 고민들과 생각들 그리고 문제를 해결해 나갔던 방법들을 서술하니 생각이 다시 한번 정리된 것 같아 아주 좋았습니다!!! 나름 잘 구현했네요.
로또 게임을 개발하다 왜 안되지 하면서 개빡쳐서 하루에 4끼 먹었던 적도 있었고 너무 집중해서 1끼 밖에 안먹은 적도 있었지만 아주 재미있었구요 무엇보다 확실한 건 제 4일 이라는 시간이 코드에 녹아난게 느껴지는것 같습니다.
이런 기회 만들어 주셔서 너무 감사할 따름이네요. 정말 너무 많이 배워가요!!! 잠만보 짱!!!!
돌

imbf · 2020-03-09T06:45:14Z

+        return lottoResult;
+    }
+
+    private void checkLotto(Lotto lotto, List<LottoRankResult> lottoRankResults) {


LottoGame내의 lotto를 check해서 Rank의 valueOf 메소드를 활용해 해당 Lotto의 Rank를 구한다음 lottoRankResults내의 Rank와 일치하면 해당 갯수를 1개 올리는 로직을 구현한 메소드 입니다. LottoResult라는 객체를 만들고 나니까 술술 풀리더군요!!

imbf · 2020-03-09T06:50:23Z

+        }
+    }
+
+    private int checkNumberMatch(List<Integer> integers, int number) {


Lotto의 LottoNumbers를 List로 변환해서 winningLotto의 lottoNumbers의 lottoNumber이 들어 있는지 체크하는 메서드이다. 안에 들어있으면 1 리턴 없으면 0을 리턴한다.

imbf · 2020-03-09T06:51:21Z

+        return 0;
+    }
+
+    private boolean isBonusMatch(List<Integer> integers) {


Lotto의 LottoNumbers를 List로 변환해 보너스 번호가 들어있는지 체크하는 메서드이다. 들어 있으면 true리턴 없으면 false리턴

imbf · 2020-03-09T06:52:50Z

+        this.number = number;
+    }
+
+    private void validateRange(int number) {


로또 번호 자체는 1이상 45이하의 자연수 이기 때문에 조건에 맞지 않는 인자가 들어오면 객체 자체를 생성조차 하지 못하게끔 만드는 메서드이다.

imbf · 2020-03-09T06:55:13Z

+    }
+
+    @Override
+    public int compareTo(LottoNumber lottoNumber) {


Lotto객체를 LottoNumber을 기준으로 정렬하기 위해 Comparable인터페이스를 구현했고 Comparable인터페이스의 추상 메소드인 compareTo를 오버라이드 했다.

imbf · 2020-03-09T07:07:43Z

+
+public class StringConverter {
+
+    public static List<Lotto> convertStringsToLottos(List<String> strings) {


여러개의 String을 여러개의 Lotto로 바꾸는 메서드이다.

imbf · 2020-03-09T07:09:49Z

+        return lottos;
+    }
+
+    public static Lotto convertStringToLotto(String string) {


멘토님께선 이 메소드를 왜 private로 쓰지 않느냐 하고 궁금해 하실 수 있는데 Production 로직을 개발하면서 String에서 Lotto로 변환이 필요한 로직이 있어서 일부러 이렇게 나누어 놓은 것이다. 원래는 스트림을 이용해서 한방에 Strings에서 Lottos로 바꾸었었다.
나름대로 재사용성을 증가했다 ㅎㅎ(?)

imbf · 2020-03-09T07:10:36Z

@@ -1,16 +1,39 @@
 package view;


Input과 Output에 대해서는 별로 고민한 건 없다!! 모델과 컨트롤러가 나오니까 뷰는 자연스레 개발되었다.

imbf · 2020-03-09T07:12:50Z

+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+public class LottoGameTest {


Mock을 사용해서 테스트를 구현해보았다!! 나름대로 불확실성을 가진 객체에 대한 의존을 밖으로 빼니까 Mock을 사용하면 쉽게 테스트 가능하다는것을 알게 되었다.

imbf · 2020-03-09T07:14:02Z

+    }
+
+    @Test
+    public void createResultTest() {


코드가 더러워 보이지만.. 사실 독자를 나름(?) 배려한것이다.. 테스트 코드에서는 가독성을 중요시 해 .(참조 연산자)를 보다 줄여보았다,

imbf · 2020-03-09T09:44:57Z

    compile('ch.qos.logback:logback-classic:1.2.3')
    testCompile('junit:junit:4.12')
    testCompile('org.assertj:assertj-core:3.9.0')
+    testCompile group: 'org.mockito', name: 'mockito-core', version: '3.3.0'


편리하게 Mock을 사용하기 위해서 mockito 프레임워크 의존성을 테스트 디렉토리에 추가했습니다.

hyukjin-lee

구현 잘 하셨네요.
고생 많이 하신게 느껴졌습니다.

피드백 조금 남겼으니 확인해주시면 감사하겠습니다.

이제 과정이 얼추 종료되어가니 한 가지 당부의 말씀드리자면,

객체지향은 현업의 수많은 상황들을 경험하지 않고서는
제대로 이해하기 어렵습니다. 그리고 정답도 없구요.
지금 저희가 하는 과정도 요구사항에 비해 많이 과한 코드인 것은 분명합니다.
그저 연습을 하기 위해 이렇게 코드를 짜는 것일 뿐이지요.
그렇기 때문에 지금 하는 것들을 '절대적인 지식이다!' '이런 코드가 짱이야!' 라고 생각하지는 마시길 바랍니다.
축구선수가 되기 위해 운동장에서 혼자 공놀이 하는 정도로 여기시고
항상 공부하실 때 본인이 알고 있는 지식에 대해 의문을 가져보고
새로운 지식과 깨달음에 열려있으시면 좋을 것 같네요.

이번 과정 정말 수고 많으셨습니다.

hyukjin-lee · 2020-03-10T03:44:05Z

+public class WinningLotto {
+
+    private Lotto lotto;
+    private int BonusNumber;


네이밍 컨벤션 미스

hyukjin-lee · 2020-03-10T03:47:19Z

+import java.util.List;
+
+import static util.StringConverter.*;
+import static view.InputView.*;


static import 를 굳이 할 이유가 있을까요

hyukjin-lee · 2020-03-10T04:00:28Z

+        }
+    }
+
+    public List<Lotto> createLottos(int count) {


Mocking 을 하지 않아도 정적 메소드로 만들 이유가 별로 없어보이지 않나요?

hyukjin-lee · 2020-03-10T04:03:16Z

+        validateSize(lottoNumbers);
+        validateDuplication(lottoNumbers);
+        this.lottoNumbers = lottoNumbers;
+        sortLottoNumbers(this.lottoNumbers);


음 생성자에서 sort 도 하는 이유가 있나요?

음.. 없죠..ㅋㅋㅋㅋㅋ 하하

hyukjin-lee · 2020-03-10T04:04:21Z

+                .collect(Collectors.toList()));
+        if (integers.size() != LOTTO_NUMBER_COUNT) {
+            throw new IllegalArgumentException("로또 번호는 중복될 수 없습니다.");
+        }


애초에 set 을 사용했으면 불필요한 변환과 검증작업이 필요 없어지지 않을까요?

Set을 사용하면 불필요한 변환과 검증 작업이 없어진다기 보다는
Lotto 클래스의 불필요한 변환과 검증작업이 User에게서 입력을 받는 단의 근처(Converter, Application)로 옮겨 지겠네요. (1,2,3,4,5,6,6과 1,2,3,4,5,6 입력이 Set에 들어가면 동일해지기 때문에 무조건 Set으로 변환하기전과 후에 Set의 Size를 검사해야한다.)

그렇다면 Lotto클래스보다 Converter나 Application에서 유효성검사를 진행하는게 보다 더 낫나요??
(저는 개인적으로 큰 차이는 없다고 생각합니다. 그치만 초보의 생각으로써 왠지 Lotto 클래스에서 유효성 검사를 하고 예외를 발생하는게 좀 더 맞다는 느낌적인 느낌이..(자바 왕초보 생각))

음.. 필드로 가지고 있는 lottoNumbers 를 Set 으로 구현하는 것에 대한 이야기였는데
말씀해주신 내용을 잘 이해하지 못했습니다. 혹시 추가설명 가능하실까요

음... 만약
User에게서 1,2,3,4,5,6,6와 같은 입력이 주어질 경우

이를 바로 Set으로 변환한다면 1,2,3,4,5,6 요소를 가진 Set이 되버립니다!!
하지만 1,2,3,4,5,6,6이라는 입력값 자체가 틀리기 때문에 저희 어플리케이션은 예외를 발생해야만 합니다.

그렇다면 lottoNumbers를 Set으로 변환하기전 List또는 Array로 변환해서 중복검사와 사이즈검사를 수행해야되기때문에 변환과 검증작업이 무조건 필요하고 결국에는 Lotto클래스에서 LottoMachine클래스로 변환과 검증 작업이 옮겨진다고 생각했습니다!!

그래서 LottoMachine에서 검증된 데이터에 대해서만 Lotto객체를 무조건 생성할 것인가??
아니면 Lotto객체 자신이 데이터를 검증해서 검증된 데이터에만 Lotto객체를 생성할것인가?
라는 문제로 좁혀지는것 같은데 저는 왠지 LottoMachine에서 변환과 검증이 있기 보다는 Lotto에서 변환과 검증이 존재했으면 좋겠다는 생각이여서 말씀드렸씁니다!!

hyukjin-lee · 2020-03-10T04:20:56Z

+
+public class LottoNumber  implements Comparable<LottoNumber>{
+
+


불필요한 공백

hyukjin-lee · 2020-03-10T04:25:24Z

+            return Rank.FOURTH;
+        }
+        if (countOfMatch == 3) {
+            return Rank.FIFTH;


Rank 배열같은걸 선언해두고 index 로 접근하면 if 를 줄일 수 있지 않을까요

hyukjin-lee · 2020-03-10T04:28:06Z

+import java.util.List;
+import java.util.stream.Collectors;
+
+public class StringConverter {


메서드들이 모두 static 인 이유가 있나요

현재 요구사항에서는 자주 사용되지 않지만 요구사항이 많아질 경우 자주 사용될 수도 있다고 생각되어 util 패키지에 해당 클래스를 위치시켰습니다.

util 패키지에 존재하는 클래스들은 자주 사용될 수도 있다고 가정해놓았기 때문에 사용할 때마다 객체를 생성하는게 비효율적이라고 생각되어 메모리를 아끼기 위해서 정적 메소드로 만들었습니다.

저 또한 이 메소드를 작성하면서 도대체 어디까지 생각해야 하는걸까 하고 고민했습니다만.. 결과적으로는 정적 메소드로 생성했습니다.

LottoMachine 클래스를 만들었기 때문에 이제는 필요 없네용~!

static 은 어플리케이션의 시작부터 종료까지 메모리를 차지하므로
사용할 때마다 객체를 생성하는 것이 더 효율적일 수 있습니다.
가비지컬렉션이 있으니깐요 ^^ 어떨 때 static 을 활용한 util 클래스를 만들어야하는지도 고민해보시면 좋을 것 같아요.
util 은 다른 라이브러리나 프레임워크에서도 많이 쓰이니까 공통점을 찾다보면 금방 감이 올 수도 있습니다.

아주 좋은 정보 감사드립니다!!!
Static 메서드라는게 매우 쉽게 쓰는것 같지만 또 매우 어렵네요!!!
많이 공부해보겠습니다!!

hyukjin-lee · 2020-03-10T04:33:21Z

+
+public class LottoNumberTest {
+
+    @Rule


rule 을 사용할 필요가 있나요?
저번처럼 assertThrow 하면 될 것 같은데

hyukjin-lee · 2020-03-10T04:36:17Z

+import java.util.ArrayList;
+import java.util.List;
+
+public class LottoResult {


DTO는 아닙니다.
DTO 는 데이터의 역할만해서 getter setter 밖에 없습니다.
DTO 를 의도하신거면 비즈니스 로직을 다른 곳에 위임해보는 게 어떨까요

hyukjin-lee · 2020-03-10T04:56:35Z

아 아 마이크 테스트

imbf · 2020-03-10T04:57:02Z

잘들려요

hyukjin-lee · 2020-03-16T12:32:06Z

+                .collect(Collectors.toList()));
+        if (integers.size() != LOTTO_NUMBER_COUNT) {
+            throw new IllegalArgumentException("로또 번호는 중복될 수 없습니다.");
+        }


음.. 필드로 가지고 있는 lottoNumbers 를 Set 으로 구현하는 것에 대한 이야기였는데
말씀해주신 내용을 잘 이해하지 못했습니다. 혹시 추가설명 가능하실까요

hyukjin-lee · 2020-03-16T12:43:43Z

+    }
+
+    public void setWinningLotto(WinningLotto winningLotto) {
+        this.winningLotto = winningLotto;


set set set get get get 을 하면 절차지향적인 프로그래밍을 하는 것이고,
setter의 유무로 객체의 상태가 immutable 하다는 확신을 할 수 없어지기 때문에
대규모의 코드에서는 쉽게 코드를 변경하지 못 하게 되는 단점이 생깁니다. 그리고 상태값이 여기저기서 변경되면 장애날 확률이 높아지기도 하구요.
그리고 메서드의 입력이 없고 입력으로 들어갈 수 있는 것을 굳이 인스턴스 변수로 선언하고 set 을 하게 된다면,
메서드를 쉽게 재사용하기 힘들어집니다. '어떤 input 이 주어지면 이 메서드가 어떤 result 를 반환한다.' 이런 예상을 하고 다른 개발자가 사용을 할 텐데, 이 메서드를 사용하기 위해서는 setter 가 선행되어야 하는 암묵적인 규칙이 생기는거고, 그런 규칙에 대해 모르고 코딩을 하면 결국 장애로 이어질 수 밖에 없습니다.
객체에 상태값 저장이 필요한 경우와 필요하지 않은 경우가 있는데, 이 경우에는 꼭 필요한가? 싶어서 여쭤본거였습니다 👍

hyukjin-lee · 2020-03-16T12:49:12Z

+import java.util.List;
+import java.util.stream.Collectors;
+
+public class StringConverter {


static 은 어플리케이션의 시작부터 종료까지 메모리를 차지하므로
사용할 때마다 객체를 생성하는 것이 더 효율적일 수 있습니다.
가비지컬렉션이 있으니깐요 ^^ 어떨 때 static 을 활용한 util 클래스를 만들어야하는지도 고민해보시면 좋을 것 같아요.
util 은 다른 라이브러리나 프레임워크에서도 많이 쓰이니까 공통점을 찾다보면 금방 감이 올 수도 있습니다.

imbf · 2020-03-17T00:19:39Z

[Lotto Game Refactoring]
리뷰 반영해서 멋지게 리팩토링 해보았습니다!!
항상 감사합니다 잠만보님 ^o^

imbf added 2 commits March 8, 2020 23:08

Creates Production Codes

8fd2e1b

Complete Production Codes

02bdf8d

imbf commented Mar 9, 2020

View reviewed changes

hyukjin-lee requested changes Mar 10, 2020

View reviewed changes

imbf requested a review from hyukjin-lee March 10, 2020 09:52

hyukjin-lee requested changes Mar 16, 2020

View reviewed changes

imbf added 2 commits March 17, 2020 09:14

Refactor Lotto Game

6773c66

Refactor Lotto Game

823e2c3


		private List<Integer> integers;

		public AutoLottoGenerator() {


		private List<LottoNumber> lottoNumbers;

		public Lotto(List<LottoNumber> lottoNumbers) {


		public class LottoConverter {

		public static List<Integer> convertLottoToIntegers(Lotto lotto) {


		public class StringConverter {

		public static List<Lotto> convertStringsToLottos(List<String> strings) {


		public class LottoNumber implements Comparable<LottoNumber>{

Uh oh!

Conversation

imbf commented Mar 9, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

imbf Mar 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

imbf left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hyukjin-lee left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

imbf Mar 9, 2020 •

edited

Loading

hyukjin-lee left a comment •

edited

Loading