Conversation
|
안녕하세요 병주님 말씀해주신 부분 중점적으로 보고 리뷰 해보도록 하겠습니다! |
| ArrayList<LottoNumber> numbers = Arrays.stream(sc.nextLine().split(", ")) | ||
| .map(x -> new LottoNumber(Integer.parseInt(x))) | ||
| .collect(Collectors.toCollection(ArrayList::new)); | ||
|
|
There was a problem hiding this comment.
로또 번호 하나를 LottoNumber 객체를 통해서 원시값을 포장하려고 해주신 것 같아요.
LottoNumber 클래스가 검증 및 예외 처리 ( 1~45 이내 숫자만 포함하는지 ) 기능을 갖고 있다면 더 좋을 것 같습니다!
https://velog.io/@kanamycine/Java-%EC%9B%90%EC%8B%9C%EA%B0%92-%ED%8F%AC%EC%9E%A5
위 블로그에 같은 로또 번호 검증 및 예외 처리가 있더라구요 참조해보셔도 좋을 것 같아요
| .map(LottoNumber::getNumber) | ||
| .noneMatch(num -> num == returnNum)) { | ||
| // `returnNum`과 일치하는 숫자가 없으면 true 반환 | ||
| lottoNumbers.add(new LottoNumber(returnNum)); |
There was a problem hiding this comment.
isNotInList 메서드를 통해 로또 번호가 중복되는 번호 없게끔 확인하기 위해서 검증하는 부분을 넣어주신 점이 좋습니다.
다만 검증에서 통과하면 메서드 내부에서 로또 번호 랜덤으로 생성하여 추가하고 있는데 이 부분을 분리해주시면 좋을 것 같아요.
isNotInList 메서드에서는 이름 처럼 검증만 하여 boolean으로 검증에 통과했는지 실패했는지 반환해주고 그 외부에서 번호를 추가해주는 것이 좋을 것 같습니다
| public class LottoController { | ||
| public static void main(String[] args) { | ||
| Lottos lottos = new Lottos(); | ||
|
|
There was a problem hiding this comment.
MVC 패턴에 기반하여 Controller, Model, View 나누어주신 것 같아요!
다만 main 함수가 있는 부분은 따로 빼주시는게 좋아보입니다! Controller가 LottoApplication의 main 함수 안에 존재하는 것이 맞을 것 같아요
| FIVE_MATCHES(5, false, 1500000), | ||
| FIVE_MATCHES_BONUS(5, true, 30000000), | ||
| SIX_MATCHES(6, false, 2000000000); | ||
|
|
There was a problem hiding this comment.
Enum을 활용하여 당첨 상금 및 통계에 관하여 관리할 수 있도록 해주신 점 매우 좋은 것 같습니다
| public int getPrice() { | ||
| return price; | ||
| } | ||
|
|
There was a problem hiding this comment.
Lotto 객체에서 getPrice() 메서드는 사용되지 않은 것 같아요. 마찬가지로 각 Lotto 객체가 price 필드를 가질 필요 또한 없어보입니다.
병주님께서 Constants 클래스로 이미" LOTTO_PRICE = 1000 "이라고 따로 관리해주시고 있기 때문에 중복되는 내용인 것 같아요!
price 필드를 지우게 되면 Lotto() 생성자의 매개변수 또한 줄어들면서 조금 더 가독성이 좋아지는 코드가 될 것 같습니다
| public void validateLottos(ArrayList<LottoNumber> lottoNumbers) { | ||
| sizeCheck(lottoNumbers); | ||
| duplicationCheck(lottoNumbers); | ||
| } |
| int numHand = sc.nextInt(); | ||
| sc.nextLine(); | ||
| handLotto(lottos, numHand); | ||
|
|
There was a problem hiding this comment.
getHandLotto()는 InputView객체의 목적에 맞게 사용자로부터 구매할 로또 수만을 입력 받도록 하는게 좋을 것 같아요.
getHandLotto()가 호출한 handLotto() 메서드에서는 수동으로 구매할 번호를 입력받아 즉시 Lotto 객체를 생성하여 lottos에 추가하고 있는데 이 부분은 모두 분리될 필요가 있어보입니다.
getHandLotto() : 사용자로부터 구매할 로또 수 입력
handLotto() : 수동 번호 입력
아래는 가상의 메서드입니다.
makeHandLotto() : 입력 받은 수동 번호로부터 Lotto 객체 생성
위와 같이 메서드의 기능을 분리하여 각 메서드가 하나의 역할만을 수행하도록 하고 이 메서드들을 Controller가 활용하도록 하는 것이 객체 지향 프로그래밍에 더욱 알맞을 것 같아요 !
2 participants
일정때문에 시간이 부족해서 코드가 잘 짜여지지는 않은 것 같습니다.
그 점 양해해 주세용ㅠ
봐주시면 좋을 것 같습니다.