로또 - 2단계 PR요청드립니다 #4220
로또 - 2단계 PR요청드립니다 #4220
Conversation
javajigi
left a comment
javajigi
left a comment
There was a problem hiding this comment.
지금 시점에 빠르게 리뷰 요청한 것 잘 했네요. 👍
질문한 부분과 관련한 피드백 남겼으니 추가 학습해 보면 좋겠습니다.
특히 객체를 너무 수동적인 존재로 바라보는데요.
이와 같이 사고하게 되는 이유에 대해 https://vimeo.com/1137582691 영상에 있으니 한번 더 추가 학습해 보면 좋겠어요.
객체를 무엇을 하는지 사고하다보니 자꾸 클래스 이름에 er을 붙이고, 수동적인 존재가 되도록 사고하는 것 같아요.
| import java.util.stream.IntStream; | ||
|
|
||
| public class Lotto { | ||
| private static final List<Integer> DEFAULT_NUMBERS = IntStream.rangeClosed(1, 45).boxed().collect(Collectors.toUnmodifiableList()); |
There was a problem hiding this comment.
LottoGenerator를 분리하지 않았으면 랜덤으로 6개 값을 생성하는 부분이 이곳에 있어도 괜찮을 것 같음.
그런데 LottoGenerator의 역할이 Lotto를 생성하는 것이라면 이 역할을 LottoGenerator로 위임하고 Lotto를 6개의 값을 가지고 로직을 실행하는 측면에 집중하는 것은 어떨까?
| private Map<Integer, Integer> lottoResult = new HashMap<>(); | ||
|
|
||
| public LottoChecker() { | ||
| lottoResult.put(3, 0); | ||
| lottoResult.put(4, 0); | ||
| lottoResult.put(5, 0); | ||
| lottoResult.put(6, 0); | ||
| } |
| int count = 0; | ||
| for (Integer number : lotto.numbers()) { | ||
| if (winning.numbers().contains(number)) { | ||
| count++; | ||
| } | ||
| } | ||
| return count; |
There was a problem hiding this comment.
대표적인 절차지향 개발임
lotto에서 계속해서 값을 꺼내고 있음.(lotto.numbers(), winning.numbers())
값을 꺼내지 말고 lotto 객체에 메시지를 보내 구현해 본다.
|
|
||
| public class LottoGenerator { | ||
| private static final long LOTTO_PRICE = 1000; | ||
| private final long price; |
There was a problem hiding this comment.
최소한의 코드로 구현했으니 "규칙 3: 모든 원시값과 문자열을 포장한다." 적용해 리팩터링
| public class LottoGenerator { | ||
| private static final long LOTTO_PRICE = 1000; | ||
| private final long price; | ||
| private final List<Lotto> lottos = new ArrayList<>(); |
There was a problem hiding this comment.
최소한의 코드로 구현했으니 "규칙 8: 일급 콜렉션을 쓴다." 적용해 리팩터링
| LottoChecker lottoChecker = new LottoChecker(); | ||
| String[] split = InputView.initWinningLotto().split(","); |
There was a problem hiding this comment.
Lotto winningLotto = LottoGenerator.generate(InputView.initWinningLotto());
위와 같이 구현해 보면 어떨까?
| import java.util.ArrayList; | ||
| import java.util.List; | ||
|
|
||
| public class LottoGenerator { |
There was a problem hiding this comment.
이 객체의 역할을 정의한 후 AI에게 이름 추천을 받아본다.
혹시라도 더 적합한 이름이 있다면 리팩터링해본다.
| @Test | ||
| @DisplayName("내가 산 복권과 당첨 복권이 3개 일치") | ||
| void countMatch() { | ||
| Lotto lotto = new Lotto(List.of(1, 2, 3, 4, 5, 6)); |
There was a problem hiding this comment.
| Lotto lotto = new Lotto(List.of(1, 2, 3, 4, 5, 6)); | |
| Lotto lotto = new Lotto(1, 2, 3, 4, 5, 6); |
Lotto를 생성하는 개발자 입장에서 위와 같이 구현할 수 있도록 지원하면 편리하지 않을까?
항상 객체와 메서드를 구현할 때 이 객체와 메서드를 사용하는 개발자 입장을 고려하면서 구현해 보는 습관을 들일 것을 추천해본다.
힌트: 가변인자
| List<Lotto> lottos = new ArrayList<>(); | ||
| lottos.add(new Lotto(List.of(1, 2, 3, 10, 11, 12))); | ||
| lottos.add(new Lotto(List.of(1, 2, 11, 12, 13, 14))); |
There was a problem hiding this comment.
| List<Lotto> lottos = new ArrayList<>(); | |
| lottos.add(new Lotto(List.of(1, 2, 3, 10, 11, 12))); | |
| lottos.add(new Lotto(List.of(1, 2, 11, 12, 13, 14))); | |
| List<Lotto> lottos = List.of(new Lotto(1, 2, 3, 10, 11, 12), new Lotto(1, 2, 11, 12, 13, 14)); |
위와 같이 List.of를 사용하는 것을 추천
코드의 간결함 이외에도 이유가 있는데 왜 그런지는 AI를 통해 피드백을 받아본다.
| import java.util.List; | ||
| import java.util.Map; | ||
|
|
||
| public class LottoChecker { |
There was a problem hiding this comment.
lottoResult에 대한 일급 콜렌션이다.
이름이 LottoResult가 더 적합하지 않을까?
이 객체는 단순히 LottoResult 값을 Map으로 가지고 이 결과 값과 관련한 책임만 가지도록 구현해 본다.
countMatch와 같은 책임은 적합해 보이지 않음
그렇다보니 Lotto의 값을 자꾸 꺼내는 방식으로 사고하게 됨
https://vimeo.com/1137582691 영상을 통해 객체에 이름을 부여하고, 객체를 좀 더 능동적인 존재로 바라보는 것이 무엇인지 고민해 보면 좋겠다.
2 participants
안녕하세요, 사실 아직 완성이 되지 않았습니다.
강의 전에 피드백 한번 받고, 그 점을 생각하고 라이브 강의 듣는게 좋을거 같아서 요청드립니다.
이번엔 조금 더 고민을 해봤습니다.
너무 세세하게 나누지는 않아도, 핵심적인 역할을 구분해야겠다고 생각했습니다.
domain을 크게 로또를 생성하는 부분과, 로또 당첨 여부를 계산하는 쪽으로 생각했고
생성하는 부분에서 Lotto와 LottoGenerator로 / 계산하는쪽에서 LottoChecker와 LottoResult로 생각해보았습니다.
제가 추가해야할거 같은 부분은 Lotto의 numbers를 일급컬렉션으로, LottoChecker의 Map을 LottoResult로 구현하는 부분입니다.
(사실 List와 Map<Integer, Integer>으로 먼저 빠르게 구현하고 리팩토링하려고 했는데, 중간에 일이 생겨.. 시간이 생각보다 걸렸네요ㅠ)
특히 LottoResult는 수정하지 않으면 마지막 출력을 완전 하드코딩적으로 해야하는거 같아서 아찔했습니다.
LottoResult를 Map<enum, Integer>와 같이 구현해서 일급컬렉션으로 가는 방향을 고민해봤는데 괜찮을까요?
이외의 부분도 피드백주시면...강의전에 최대한 생각해보고 일부 구현해보겠습니다.