-
Notifications
You must be signed in to change notification settings - Fork 47
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[로또 미션] - 오채현 미션 제출합니다. #3
base: che7371
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
전체적으로 확인해 봤을때 중점적으로 보라고 하셨던 4가지들이 대부분 잘 지켜진 것 같아요😀
그런데 입력에 대한 예외 처리가 되어 있지 않아 예외 처리가 필요해 보이는 것 같아요.
예외 처리 해야되는 것들의 예시
- Price: (범위:0이상, 형식: Integer, 단위:1000)
- Lotto안의 숫자들(범위:1~45, 형식: Integer)
- Lotto: (중복x, 숫자개수:6개)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
헉 예외 처리를 안 하고 넘어갔네요. 죄송합니다. 반영해서 수정하겠습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lotto의 numbers를 List로 구현하셨는데, 4단계 힌트에서 나와있는 것처럼 로또 숫자 하나를 표현하는 Integer를 포장한 LottoNumber 객체를 구현하시면 예외처리할때 도움이 될 것 같아요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
딱 필요한 함수들과 변수들로 깔끔하게 잘 짜신 것 같아요!
putPrize(matches,bonusMatches,stats); | ||
} | ||
double revenue=calculateRevenue(stats,lottoList); | ||
outputView.printWinningResult(stats,revenue,prize); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
model인 LottoService에서 OutputView를 호출하는 것보다
LottoController에서 따로 메소드를 만들어 호출하는 것이 일관성에 더 좋을 것 같아요
public int calculateBonusBallMatch(Lotto lotto,int matches,int bonusBall){ | ||
return matches==Match.MATCH_5.getMatch() && lotto.getNumbers().contains(bonusBall) ? 1 : 0; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
프로그래밍 요구사항에 삼항 연산자를 쓰지 말라고 적혀있더라고요.
아마도 가독성이 떨어져서 그런것 같아요.
함수를 추출해서 처리해보는 것이 어떨까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저도 enum을 활용하는데 어려움을 겪었는데, 필요한 부분에 깔끔하게 잘 쓰신 것 같아요:)
lottoNums = new ArrayList<>(); | ||
for (int i = 0; i < CNT_LOTTO_NUMBER; i++) { | ||
int uniqueNumber = numbers.get(i); | ||
while (lottoNums.contains(uniqueNumber)) { | ||
Collections.shuffle(numbers); | ||
uniqueNumber = numbers.get(i); | ||
} | ||
lottoNums.add(uniqueNumber); | ||
} | ||
Collections.sort(lottoNums); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
랜덤으로 숫자를 뽑으실때 이렇게 하시면
List lottoNums = numbers.subList(0, 6);
Collections.sort(lottoNums);
더 간단하게 하실 수 있을 것 같아요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
메소드 순서대로 아래에 구현돼있어서 읽기 편했어요:)
int manualPrice= inputManual(); | ||
viewLotto(price,manualPrice); | ||
List<Integer> winningNums=inputView.inputWinningNumbers(); | ||
lottoList.setTotalPrice(price);//넣어야함 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
꼭 필요한 주석이 아닌 것 같아 지우셔도 될 것 같아요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Map 사용한 Prize로 구현하신것도 괜찮게 잘 짜신 것 같아요.
ENUM에서 내부 매소드를 이용해서 enum의 정보를 이용하는 방법도 있어요.
예시로 ordinal이라는 함수를 이용하면 몇번째로 구현됐는 지도 알려줘서 따로 리스트를 만들어서
index로 저장하는 방법이 있습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ENUM Match 클래스 짜놓고 활용 방법이 안 떠올라서 결국 Prize 클래스를 이용했는데 그런 방법이 있군요..! 감사합니다. 관련해서 더 찾아봐야겠어요. 아직 자바 문법 지식이 많이 부족하네요. 열심히 노력하겠습니다 !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
view는 너무 이쁘게 잘 짜신 것 같아요:)
1단계 구현 완료, 2단계 구현 완료, 3단계 구현, 4단계 구현 이렇게 봐주시면 됩니다.
3단계 구현 부분에서 새로운 프로그래밍 요구사항으로 Java Enum을 사용하라는 말이 있었습니다. 그래서 제가 Match 클래스는 작성을 해놓은 상태인데, 이걸 Prize랑 연결지어서 써보려고 했는데 제 실력 부족+이해도 부족으로 실패했습니다...아직 Enum에 대한 이해도가 많이 부족한 것 같아서 따로 공부를 해야할 것 같습니다. 죄송합니다. 일단 Map 사용한 Prize로 구현은 해놓았습니다...! 참고하고 봐주시면 감사하겠습니다.
이 부분 중점적으로 봐주시면 될 것 같습니다!
->이 4가지는 제가 원래부터 이해가 부족한 부분이어서 많이 헤매면서 진행했습니다. 그래서 코드가 갑자기 더러워진다거나 이렇게 구현하면 요구사항에 어긋나는데? 하는 부분이 많습니다.. 감안하고 봐주시면 감사하겠습니다! :-)