[2주차] 로또 - 클린코드 #65
[2주차] 로또 - 클린코드 #65
Conversation
| public class InputFromUser { | ||
|
|
||
| private static final Scanner scanner = new Scanner(System.in); |
There was a problem hiding this comment.
Scanner를 이용한 입력 부분을 따로 분리해주셨군요! InputView가 존재하지만, 이를 한번 더 분리해주신 이유가 있을까요?
There was a problem hiding this comment.
저는 '사용자 입력'과 '입력문구 출력'이라는 역할을 분리해서 보고 싶었습니다!! 그리고 이후 입력단계에서 잘못된 입력에 대해서 예외처리를 쉽게 할 수 있도록 하기 위해서 이렇게 분리했습니다!!
| @@ -0,0 +1,25 @@ | |||
| package view; | |||
There was a problem hiding this comment.
저 개인적으로는, view 계층은 사용자와 가장 맞닿아 있는 곳이라 생각하는데요! 의진님이 생각하신 view 패키지의 역할은 무엇인가요?
There was a problem hiding this comment.
저는 view 패키지의 역할은 사용자에게 정보를 제공하는 패키지라고 생각합니다!! 사용자가 어떤 행동을 해야하고, 그에 따라 어떤 결과를 얻을 수 있는지 정보를 제공하는 패키지라고 생각합니다 : )
| @@ -0,0 +1,32 @@ | |||
| package util; | |||
There was a problem hiding this comment.
util 패키지는 무엇을 담을 수 있는 패키지인가요? 어쩌면 이것저것 다 담을 수 있는 만능 패키지가 될 수도 있을 것 같다는 우려가 있어요.
패키지를 나누는 이유가 무엇인지 고민해보면 좋을 것 같습니다.
There was a problem hiding this comment.
저도 이번에 util에 대해서 고민을 많이 해보았는데... 프로그램 전체에 걸쳐서 필요한 기능을 util 패키지에 넣는 것이 맞는 것 같다고 생각했습니다!!(ex : 로또 번호 변환 -> 우승로또 입력, 수동로또 입력에 사용 등)
| @@ -0,0 +1,54 @@ | |||
| # 🚀1단계 - 로또 자동구매🚀 | |||
There was a problem hiding this comment.
리드미를 잘 작성해주셨네요! 👏
요구 사항과 구현 사항을 정리했을 때, 의진님이 느끼신 장점이 있었나요?
There was a problem hiding this comment.
네 있습니다! 이전에는 이 기능, 저 기능 순서없이 구현하다보니 나중에 보면 주석이 있음에도 제가 이해하기 힘든 코드가 많았습니다. 하지만 이렇게 요구 사항과 구현 사항을 정리했을 때 제가 의도한 대로 코드를 작성했는지 쉽게 볼 수 있었고, 이후 추가적인 구현기능이 있더라도 비교적 쉽게 추가할 수 있었던 것 같습니다 : )
|
|
||
| public class LottoCalculator { | ||
|
|
||
| private static final int LOTTO_COST = 1000; |
There was a problem hiding this comment.
로또 가격을 상수로 추출해 주셨군요! 적절한 이름을 붙여주셔서, 코드를 읽는 입장에서 1000 이라는 숫자의 의미를 이해하는데 수월했습니다 👍
로또 가격에 대한 관리 포인트를 한 곳으로 줄인다는 관점에서도 좋은 것 같아요.
| } | ||
|
|
||
| public boolean checkBonusNumber(final WinningLotto lastWeekWinnerLotto, final Lotto lotto) { | ||
| return lotto.getLottoNumber().contains(lastWeekWinnerLotto.getBonusNumber()); |
There was a problem hiding this comment.
위의 리뷰와 동일한 맥락입니다.
https://mangkyu.tistory.com/147
There was a problem hiding this comment.
여기도 객체의 역할로 정할 수 있었네요! 감사합니다 : )
| final int passiveLottoCount = 2; | ||
|
|
||
| //when | ||
| final int result = lottoCalculator.calculateAutoLottoCount(buyingCosts, passiveLottoCount); |
There was a problem hiding this comment.
테스트 작성 시에, 실제로 나온 값에 대한 네이밍은 관례적으로 actual 이라는 이름을 붙이는 것 같아요. 네이밍은 정답이 없는 분야이니, 적당히 참고해주시면 될 것 같아요!
There was a problem hiding this comment.
아하! 관례를 따라가는 것이 이후 좋은 영향이 있을 거 같습니다 감사합니다 : )
| List<Integer> result = LottoNumberConvertor.convertLottoNumbers(lottoNumbers); | ||
|
|
||
| //then | ||
| Assertions.assertThat(result).usingRecursiveComparison().isEqualTo(expected); |
| Assertions.assertThat(WinningLottosStatus.of(correctCount, isSecondPrize).getLottoCount()).isEqualTo(expected); | ||
| } | ||
|
|
||
| static Stream<Arguments> generateData() { |
There was a problem hiding this comment.
👍 👏👏
@ParameterizedTest를 잘 활용해주셨네요.. 의진님 고수시군요
There was a problem hiding this comment.
아닙니다... 모두 보고 배운 겁니다 너무 감사합니다 : )
| @BeforeEach | ||
| public void initializeWinningLotto() { | ||
| WinningLottosStatus.initialize(); | ||
| } |
There was a problem hiding this comment.
이 부분에 대한 생각을 남겨보자면,
일단, 매번 초기화를 해줘야 하는 이유부터 파악을 해야겠죠. 가장 근본적인 이유는, 싱글턴으로 존재하는 Enum 인스턴스에 lottoCount 라는 변화하는 값을 필드로 넣은 것이 문제의 출발점입니다.
그렇다면 Enum 안에 있는 lottoCount를 제거하면 되지 않을까요? 이를 계산해서 저장하는 곳을 따로 만드는 것이 좋을 것 같아요.
추가로, Enum의 역할에 대해 다시 생각해 볼 필요가 있는 것 같아요. Enum에 변수 값이 들어간 순간부터, 이를 더 이상 상수라는 개념으로 보기 어려울 것 같습니다.
There was a problem hiding this comment.
정확하게 말씀해 주신 것 같습니다.. Enum은 변하지 않는 상수여야 하는데 업데이트를 하는 것이 맞는 것인가 생각을 했었습니다. 답이 의외로 간단했군요..! 감사합니다, 수정해보도록 하겠습니다 : )
안녕하세요!! 바쁘신 스케줄 중에도 시간 내어 리뷰해주셔서 정말 감사합니다 : ) 많이 배우도록 하겠습니다..!!
🚀 기능 구현 🚀
❓ 질문 사항 ❓
🔧 수정 사항 🔧