Conversation
| this.lottos = Collections.unmodifiableList(new ArrayList<>(lottos)); | ||
| this.lottoCount = lottoCount; |
There was a problem hiding this comment.
Collections.unmodifiableList를 사용하셨군요!
무슨 기능을 하는 지 몰랐는데 이번 기회에 공부할 수 있어서 좋았습니다.
저는 이번에 Java를 배우기 시작하면서 이전에 몰랐던 새로운 요소들을 봤을 때, 그리고 썼을 때 항상 굳이? 라는 생각을 먼저 하고 시작하는데요, 혹시 여기서 unmodifiableList를 사용하신 이유가 궁금합니다 !
| private static List<Lotto> createManualLottos(List<String> manualLotto, LottoCount count) { | ||
| return manualLotto.stream() | ||
| .map(Lotto::of) | ||
| .collect(Collectors.toList()); | ||
| } |
| private static void DisposeBall() { | ||
| for (int ballNumber = MINIMUM_LOTTO_NUMBER; ballNumber <= MAXIMUM_LOTTO_NUMBER; ballNumber++) { | ||
| MAPPING_BALL.put(ballNumber, new LottoNumber(ballNumber)); | ||
| } | ||
| } |
There was a problem hiding this comment.
로또 넘버들을 초기화하는데 Map을 사용하셨군요, 좋은 아이디어 같습니다!
오늘 세션에서처럼 이같은 경우 Map을 사용하면 중복을 제거하는 로직을 넣지 않아도 된다는 이점이 있을 것 같네요.
하지만 for문을 통해 횟수를 45회로 제한한다면 이와 같은 기능이 충분이 잘 동작할 수 있을까요?
만약 중간에 중복된 값이 들어간다면 44개의 공만 저장되나요?
45개의 숫자가 확실하게 들어온다고 가정한다면 Map을 쓸 이유가 있을까요?
| THREE_MATCH(new MatchCount(3), false, new Money(5_000)), | ||
| FOUR_MATCH(new MatchCount(4), false, new Money(50_000)), | ||
| FIVE_MATCH(new MatchCount(5), false, new Money(1_500_000)), | ||
| FIVE_MATCH_WITH_BONUS_BALL(new MatchCount(5), true, new Money(30_000_000)), | ||
| SIX_MATCH(new MatchCount(6), false, new Money(2_000_000_000)); |
There was a problem hiding this comment.
_로 읽기쉽게 구분할 수 있었군요..!
좋은 것 배워갑니다 ! 👍
| THREE_MATCH(new MatchCount(3), false, new Money(5_000)), | ||
| FOUR_MATCH(new MatchCount(4), false, new Money(50_000)), | ||
| FIVE_MATCH(new MatchCount(5), false, new Money(1_500_000)), | ||
| FIVE_MATCH_WITH_BONUS_BALL(new MatchCount(5), true, new Money(30_000_000)), | ||
| SIX_MATCH(new MatchCount(6), false, new Money(2_000_000_000)); |
There was a problem hiding this comment.
Enum값에서 1, 3번째 값은 새로운 객체를 생성해서 초기화 한다는 게 흥미롭네요!!
하지만 입력받는 값이나 전달받는 값이 아닌 상수를 정의한다는 점에서 굳이 이런 방식을 사용하신 이유가 있나요?
궁금합니다 👍
| public float getProfitRate() { | ||
| float totalPrize = lottoResult.calculateTotalPrize(); | ||
| Money money = Money.getMoney(count); | ||
| return totalPrize / money.getMoney(); | ||
| } |
There was a problem hiding this comment.
저는 실수형을 사용할 때 최대한 float대신 double을 사용하고있습니다.
실수형의 default는 double이기 때문에요!
| public boolean isProfit() { | ||
| if (getProfitRate() < PROFIT_THRESHOLD) { | ||
| return false; | ||
| } | ||
| return true; | ||
| } |
There was a problem hiding this comment.
이 메소드를 도대체 왜 작성하셨지 하면서 천천히 생각해보니까 1이상일 때는 해당하는 메세지를 출력하면 안되는 것이었군요 ㅠㅠ 세세한 부분까지 캐치하신 거 대단하시네요 ~!
| public Printer(){ | ||
| } |
|
수동 미션하느라 수고하셨습니다 :) 코드 리뷰하는 동안 여러 부분에서 배워갑니다, 감사합니다 ! |
kouz95
left a comment
kouz95
left a comment
There was a problem hiding this comment.
고생하셨습니다 👍
리뷰 이외에도 테스트가 작성되지 않은 부분이 몇몇 있는것 같아요.
테스트 커버리지 한번 확인해주세요 🙂 (getter 같은 부분까지 테스트 할 필요는 없습니다.)
| Printer printer = new Printer(); | ||
| Receiver receiver = new Receiver(); |
There was a problem hiding this comment.
| Printer printer = new Printer(); | |
| Receiver receiver = new Receiver(); | |
| private final Printer printer = new Printer(); | |
| private final Receiver receiver = new Receiver(); |
main이니 큰 문제는 없겠지만, 그래도 오용을 막기위해선 private final을 붙여주는게 안전할것 같네요 🙂
| public class Lottos implements Iterable<Lotto> { | ||
|
|
||
| private final List<Lotto> lottos; | ||
| private final LottoCount lottoCount; |
There was a problem hiding this comment.
lottoCount를 인스턴스 변수로 갖고 있어야만 할까요?
인스턴스 변수의 개수는 가능한 최소한으로 유지하는게 좋습니다.
(이유에 대해서도 생각해볼까요? 🤔)
| private static final int MINIMUM_MANUAL_LOTTO_COUNT = 0; | ||
| private final int manualLottoCount; |
There was a problem hiding this comment.
| private static final int MINIMUM_MANUAL_LOTTO_COUNT = 0; | |
| private final int manualLottoCount; | |
| private static final int MINIMUM_MANUAL_LOTTO_COUNT = 0; | |
| private final int manualLottoCount; |
🙂
| } | ||
|
|
||
| public boolean hasManualLottoCount() { | ||
| return manualLottoCount != 0; |
3 participants
1차 미션 진행하다가 새로 해보자는 생각에 시도했다가 이제서야 완성이 되었네요.
늦어서 죄송합니다;(