[로또 미션] 허준기 미션 제출합니다 #42
Conversation
There was a problem hiding this comment.
고생 많으셨습니다.
정말 많은 고민을 하면서 코드를 작성하신게 코드를 읽으면서 느껴지네요
그런데 너무 객체지향에 몰입해서 오히려 복잡성이 증가한 것 같은 느낌도 있는 것 같아요
코드를 계속 보면서 clean code라는 책이 생각났는데
이 영상 시간 나면 한번 보시면 좋을 것 같습니다!(제목이 좀 이상하긴 한데ㅠ 내용은 한번 봐도 좋을것 같아요)
고생 많으셨습니다 😄
| public final class Constant { | ||
|
|
||
| private Constant() { | ||
| } | ||
|
|
||
| public static final int LOTTO_NUM_COUNT = 6; | ||
|
|
||
| public static final int LOTTO_PRICE = 1000; | ||
|
|
||
| public static final int NO_REWARD = 0; | ||
|
|
||
| public static final int FIFTH_REWARD = 5000; | ||
|
|
||
| public static final int FOURTH_REWARD = 50000; | ||
|
|
||
| public static final int THIRD_REWARD = 1500000; | ||
|
|
||
| public static final int SECOND_REWARD = 30000000; | ||
|
|
||
| public static final int FIRST_REWARD = 2000000000; | ||
|
|
||
| public static final int MIN_NUM = 1; | ||
|
|
||
| public static final int MAX_NUM = 45; | ||
| } |
There was a problem hiding this comment.
무슨 느낌으로 상수용 클래스를 만들어 하나에 모았는지는 충분히 알겠는데 한 클래스에서만 사용되는 상수들도 이렇게 모아놓는 것이 좋을지 생각해보면 좋을 것 같아요~
물론 정해진 답이 있는 것은 아니지만요
저도 덕분에 더 공부하고 가네요!
There was a problem hiding this comment.
상금쪽은 한곳에서만 쓰다보니 상금 관련 클래스에 두는게 나아보이네요 👍
| public void run() { | ||
| int inputMoney = InputView.getMoney(); | ||
| int customCount = InputView.getCustomLottoCount(); | ||
| CustomLotto customLotto = new CustomLotto(InputView.getCustomLotto(customCount)); | ||
| LottoGame lottoGame = new LottoGame(inputMoney, numberGenerator, customLotto); | ||
| OutputView.printLottoes(customCount, lottoGame.getLottoList()); | ||
| WinNums winNums = new WinNums(InputView.getWinLotto(), InputView.getBonusBall()); | ||
| CheckPlace checkPlace = new CheckPlace(lottoGame, winNums); | ||
| OutputView.printStatistics(checkPlace.getResultMap()); | ||
| Reward reward = new Reward(checkPlace.getResult(), inputMoney); | ||
| OutputView.printReward(reward.getRate()); | ||
| } |
There was a problem hiding this comment.
취향 차이라고 할 수 있는지 모르겠는데 저라면 중간 중간 가독성을 위해 개행을 넣었을 것 같아요~ 어떻게 생각하시나요?
| public void run() { | |
| int inputMoney = InputView.getMoney(); | |
| int customCount = InputView.getCustomLottoCount(); | |
| CustomLotto customLotto = new CustomLotto(InputView.getCustomLotto(customCount)); | |
| LottoGame lottoGame = new LottoGame(inputMoney, numberGenerator, customLotto); | |
| OutputView.printLottoes(customCount, lottoGame.getLottoList()); | |
| WinNums winNums = new WinNums(InputView.getWinLotto(), InputView.getBonusBall()); | |
| CheckPlace checkPlace = new CheckPlace(lottoGame, winNums); | |
| OutputView.printStatistics(checkPlace.getResultMap()); | |
| Reward reward = new Reward(checkPlace.getResult(), inputMoney); | |
| OutputView.printReward(reward.getRate()); | |
| } | |
| public void run() { | |
| int inputMoney = InputView.getMoney(); | |
| int customCount = InputView.getCustomLottoCount(); | |
| CustomLotto customLotto = new CustomLotto(InputView.getCustomLotto(customCount)); | |
| LottoGame lottoGame = new LottoGame(inputMoney, numberGenerator, customLotto); | |
| OutputView.printLottoes(customCount, lottoGame.getLottoList()); | |
| WinNums winNums = new WinNums(InputView.getWinLotto(), InputView.getBonusBall()); | |
| CheckPlace checkPlace = new CheckPlace(lottoGame, winNums); | |
| OutputView.printStatistics(checkPlace.getResultMap()); | |
| Reward reward = new Reward(checkPlace.getResult(), inputMoney); | |
| OutputView.printReward(reward.getRate()); | |
| } |
|
|
||
| import lotto.message.ConsoleMessage; | ||
|
|
||
| public class InputView { |
There was a problem hiding this comment.
저는 input에다가 다 몰빵해서 구현했는데 이렇게 output을 나눠서 구현하는게 좋았겠네요ㅋㅋㅠ 👍
| public enum ConsoleMessage { | ||
| INPUT_MONEY("구입금액을 입력해 주세요."), | ||
| COUNT_LOTTO("수동으로 %d장, 자동으로 %d개를 구매했습니다."), | ||
| LAST_WIN_NUM("지난 주 당첨 번호를 입력해 주세요."), | ||
| WINNING_STATISTICS("당첨 통계"), | ||
| CHECK_SAME("%d개 일치 (%d원) - %d개"), | ||
| CHECK_SAME_2ND("%d개 일치, 보너스 볼 일치 (%d원) -%d개"), | ||
| REWARD_RATE("총 수익률은 %s입니다."), | ||
| INPUT_BONUS("보너스 볼을 입력해 주세요."), | ||
| CUSTOM_LOTTO_COUNT("수동으로 구매할 로또 수를 입력해 주세요."), | ||
| CUSTOM_LOTTO_INPUT("수동으로 구매할 번호를 입력해 주세요.") | ||
| ; |
There was a problem hiding this comment.
여기서도 저는 한번만 쓸 애들을 enum 클래스로 만드는게 좋은 방법이였을까 하는 생각이 드네요
어떻게 생각하실까요?
There was a problem hiding this comment.
메세지 자체를 따로 모아두는게 좋다고 생각해서 모아봤습니다!!
| private void validateLottoSize() { | ||
| if (lottoNums.size() != Constant.LOTTO_NUM_COUNT) { | ||
| throw new IllegalArgumentException(ErrorMessage.INVALID_LOTTO_NUM.getMessage()); | ||
| } | ||
| } | ||
|
|
||
| private void validateLottoNum(){ | ||
| for(int num : lottoNums){ | ||
| if(!(num >= Constant.MIN_NUM && num <= Constant.MAX_NUM)){ | ||
| throw new IllegalArgumentException(ErrorMessage.INVALID_NUM.getMessage()); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
요구사항에 함수 하나는 한가지 일만 하게 하라고 나와있기는 했는데 이렇게 까지 함수로 나눌 필요성이 있었나 하는 생각이 저는 드네요
숫자를 검증하는 것과 사이즈를 검증하는 것이 과연 다른 일인가? 하는 생각이 들어요
이것도 답이 있는 것은 아니지만 어디까지 함수를 나눠야 하는가 생각해보면 좋을 것 같아요
There was a problem hiding this comment.
검증이라는 역할은 같지만 검증하는 내용이 달라서 나눴는데 저도 이거 나눌때 이렇게 나눌필요까지 있을까 하는 고민이 있었는데 일단 요구사항대로 나눴습니다!
개인적으로는 그냥 묶어도 될것 같아용
| import lotto.generator.RandomNumberGenerator; | ||
| import lotto.mock.Lotto7NumberGenerator; | ||
|
|
||
| class LottoTest { |
There was a problem hiding this comment.
테스트 까지 성실하게 잘 작성해주셨네요ㅠ
급하게 몰아서 한 제 자신이 반성됩니다
코드를 짜면 짤수록 지저분해져서 힘들었네요
피드백 부탁드립니다!!!!!!!