[2주차] 객체지향 코드 연습 (Seooooo24) #32
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Scanner로 금액과 당첨 번호를 입력받는다. String으로 입력받은 당첨 번호를 SplitNum() 메서드를 사용하여 정수 리스트로 변환한다. 정수 리스트여야 Lotto 클래스에 매개변수로 넘겨줄 수 있기 때문이다.
Scanner로 금액과 당첨 번호를 입력받는다. String으로 입력받은 당첨 번호를 SplitNum() 메서드를 사용하여 정수 리스트로 변환한다. 정수 리스트여야 Lotto 클래스에 매개변수로 넘겨줄 수 있기 때문이다.
Scanner로 금액과 당첨 번호를 입력받는다. String으로 입력받은 당첨 번호를 SplitNum() 메서드를 사용하여 정수 리스트로 변환한다. 정수 리스트여야 Lotto 클래스에 매개변수로 넘겨줄 수 있기 때문이다.
BuyLotto() 메서드는 입력받은 금액을 로또 가격으로 나누어 구매한 로또 매수를 반환한다. 입력받은 금액을 로또 가격으로 나누었을 때 나머지가 0이 아니면 IllegalArgumentException를 발생시킨다.
랜덤으로 숫자를 뽑아 리스트에 넣어주는 메서드
뽑은 로또의 숫자를 사용자에게 보여주는 메서드 displayLottoNumber()
당첨 번호와 보너스 번호를 입력받고 예외를 처리한다.
당첨 번호와 사용자가 뽑은 로또 번호를 비교한다. (보너스 번호 제외)
Seooooo24
requested review from
Hoya324,
KoSeonJe and
TaetaetaE01
as code owners
Seooooo24
changed the title
Hoya324
reviewed
Sep 30, 2024
src/main/java/lotto/Application.java
Outdated
Comment on lines
14
to
16
| LottoNum lottoNum = new LottoNum(); | ||
| VLotto vLotto = new VLotto(); | ||
| CLotto cLotto = new CLotto(lottoNum, vLotto); |
There was a problem hiding this comment.
이 연관 관계를 Application이 관리하는게 맞을까?를 고민해보시면 좋을거 같아요!!
KoSeonJe
reviewed
Sep 30, 2024
src/main/java/model/WinCheck.java
Outdated
| this.userNumber = userNumber; | ||
|
|
||
| winChecker(winNumber, userNumber); | ||
| winStats(); |
There was a problem hiding this comment.
생성자 내에 비즈니스 로직과 관련된 메소드를 수행하는 게 적절하지 않은 것 같습니다. 필드를 정의하는 코드만 작성해주는 것이 좋습니다.
src/main/java/controller/CLotto.java
Outdated
| // 당첨 번호 및 보너스 번호 입력받기 | ||
| String winningNumber = vLotto.getWinningNumber(); | ||
| List<Integer> winningNumberList = lottoNum.splitNum(winningNumber); | ||
| mLotto = new Lotto(winningNumberList); |
There was a problem hiding this comment.
mLotto가 사용되고 있지 않은 것 같습니다. 무슨 목적으로 생성했던 것인지 의도가 궁금합니다!
src/main/java/model/WinCheck.java
Outdated
| userNum.retainAll(winNumber); | ||
| if (userNum.size() == 3) { | ||
| threeNum++; | ||
| } else if (userNum.size() == 4) { |
There was a problem hiding this comment.
else if 사용을 자제해주세요.
early return 방법 사용해주시길 바랍니다.
src/main/java/model/WinCheck.java
Outdated
| private int threeNum; | ||
| private int fourNum; | ||
| private int fiveNum; | ||
| private int sixNum; |
There was a problem hiding this comment.
enum 메소드로 관리해도 좋을 것 같아요. 막연하지만?? 한번 생각해보면 좋을 것 같습니다
VLotto에서 정수로 입력받은 보너스 번호도 당첨 번호처럼 사용자 로또 번호와 매치하고 fiveBonusCount를 증가시킨다.
문제에서 제시된 형식으로 로또 번호 생성 방식을 변경했다.
각 요소별 역할과 클래스별 기능 수정
메서드가 하는 일이 표현되어 있지 않은 메서드명(lottoNumber)을 적절하게 (create)변경함
숫자를 뽑아 리스트를 만들어줄 때 오름차순으로 정렬하여 반환한다.
KoSeonJe
reviewed
Oct 6, 2024
| private WinChecker winChecker; | ||
| private RateCalculator rateCalculator; | ||
| private LottoView lottoView; | ||
| private ErrorMessage errorMessage; |
There was a problem hiding this comment.
에러메시지는 공통적으로 사용되는 클래스이기 인스턴스 생성이 필요하지 않은 것 같습니다.
Comment on lines
13
to
17
| private LottoNumber lottoNumber; | ||
| private Lotto mLotto; | ||
| private WinChecker winChecker; | ||
| private RateCalculator rateCalculator; | ||
| private LottoView lottoView; |
There was a problem hiding this comment.
의존성 주입을 외부에서 해주도록 노력해주세요. LottoNum과 LottoView 이외에는 외부에서 주입되지 않고 있습니다.
의존성 주입을 외부에서 해야하는 점을 공부해보면 좋을 것 같습니다
| int lottoCount = lottoNumber.buy(money); | ||
| lottoView.displayLottoCount(lottoCount); | ||
|
|
||
| List<List<Integer>> lottosList = new ArrayList<>(); |
There was a problem hiding this comment.
변수명에 List, Set, Map과 같이 자료형 이름을 사용하는 것은 지양해야합니다.
Comment on lines
5
to
6
| public RateCalculator() { | ||
| } |
src/main/java/model/WinChecker.java
Outdated
| import java.util.List; | ||
| import java.util.ArrayList; | ||
|
|
||
| public class WinChecker { |
There was a problem hiding this comment.
winChecker라는 이름을 보면 당첨 확인하는 역할을 가진 것 같습니다.
근데, 당첨 횟수까지 저장되어 있고, 당첨 로또 번호, 사용자 로또 번호, 보너스 번호까지 저장하고 있습니다.
따로 저장 객체를 추가해 만들어주세요
Lotto 클래스 생성자 매개변수를 수정하면 안 되는 줄 알고 Lotto 클래스를 활용하지 못하고 있었다. 클래스명들을 변경하여 목적을 알기 쉽게 했고, 한 클래스에서 하는 일을 구분지었다. 생성자 매개변수를 수정하여 controller에서 인스턴스를 생성한 뒤에 split 메서드를 사용할 수 있도록 했다.
Seooooo24
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
일단 기능을 다 구현하고 클래스를 분리하려 했으나 기능이 미완성입니다... 예외 처리도 이상합니다. 테스트 코드도 처음 접해 작성하지 못했습니다. 다음 과제부턴 꼭 완성하여 제출하겠습니다. 죄송합니다.