-
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
[로또 미션] 김진주 미션 제출합니다. #12
base: nova0128
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.
안녕하세요 진주님!! 제가 풀리퀘에서 진주님 이름을 찾지 못 해서 리뷰를 안 했다가 이름 바꾸신 걸 지금 발견해서 이제 리뷰 드려요ㅠㅠ 죄송합니다
우선 MVC 패턴에 맞게 패키지를 나누고 구현해주신 점 좋았습니다! 신경 쓰신 게 보였어요!
하나 최종적으로 코멘트 드릴 건 이번 과제에 원시값 포장에 대한 부분이 추가 됐는데 로또 번호, 로또 티켓같은 값들을 클래스로 만들어서 int 같은 원시값으로 바로 사용하는 게 아니라, LottoNumber, Lotto 객체로 감싸서 사용하는 것도 고민해보시면 좋을 것 같아요!
과제 너무 수고하셨습니다!
public class Controller { | ||
public static void main(String[] args) { | ||
OutputView.inputMessage(); | ||
int inputMoney = Input.inputMoney(); |
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 intputMoney = InputView.inputMoney(); 이런식으로 사용하는 건 어떨까요?
input에 담당하는 메서드를 InputView 클래스에서 따로 구현하는 것도 좋을 것 같습니다!
import java.util.List; | ||
|
||
public class ReturnAutoLotto { //랜덤로또리스트를 반환 | ||
public static List<Integer> randomLotto(){ |
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.
이 부분은 stream 사용법을 찾아보시고 이용하시면 가독성이 더 좋아질 것 같습니다!
List<Integer> list = new ArrayList<>(); | ||
Random random = new Random(); | ||
|
||
while (list.size() < 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.
로또 개수와 로또번호의 범위를 지정하는 값들은 상수로 대체될 수 있는 값들인 것 같습니다! 스터디에서 배운대로 매직넘버를 제거하는 것도 고려해주시면 좋을 것 같습니다
|
||
|
||
OutputView.resultsMessage(); | ||
List<Integer> winnerCount = List.of(0, 6, 5, 5, 4, 3); |
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 클래스를 사용하시면 당첨금 관련 로직이 훨씬 좋아질 것 같습니다!
|
||
//불러와서 하면 더 좋을듯 | ||
public static List<Integer> resultCount(List<List<Integer>> resultArray, List<Integer> answerNumbers, int bonusNumber) { | ||
List<Integer> resultWinningList = initResultList(); |
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.
answerCount에 따른 인덱스를 미리 정의하면
if문을 각각 적어주지 않고도 구현이 가능할 것 같습니다!
import java.util.List; | ||
|
||
public class TotalLotto { | ||
public static List<List<Integer>> createResultArray () { |
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.
로또를 Integer, List 객체로 관리하셨는데, LottoNumber, Lotto 클래스를 구현하셔서 사용하셨어도 좋았을 것 같습니다!
} | ||
|
||
public static List<List<Integer>> createAddLotto (List<Integer> singleLotto, List<List<Integer>> resultArray) { | ||
resultArray.add(singleLotto); // ? |
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.
로또에 대한 예외 처리를 해주면 더 좋을 것 같습니다!
OutputView.printLottoCountInfo(manualCount, Input.inputAutoCount(inputMoney / 1000, manualCount)); | ||
|
||
|
||
for (int i = 0; i < Input.inputAutoCount(inputMoney / 1000, manualCount); i++) { |
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<Integer> winnerCount = List.of(0, 6, 5, 5, 4, 3); | ||
List<Integer> winnerPrice = List.of(0, 2000000000, 30000000, 1500000, 50000, 5000); | ||
long sum = 0; | ||
for (int i = 5; i >= 1; i--) { |
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.
for문을 사용하여 한 번에 출력을 잘 해주신 것 같습니다!
안녕하세요 리뷰어님!
저번 과제보다 mvc패턴에 더 신경써서 구현해보았습니다.
다만 public/private, 난잡한 메소드들을 다듬지 못한 채로 제출하여 고칠 부분이 아직 많은 것 같습니다.
부족한 부분은 많이 지적해주세요!
(중간에 메인브랜치로 잘못 커밋해 포크를 방금 새로 하여 커밋이 하나밖에 없습니다..)