-
Notifications
You must be signed in to change notification settings - Fork 107
[SCG] 권승원 로또 1-2단계 미션 제출합니다. #162
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
Changes from all commits
dabf6e5
4d9915f
9e5bc4a
bf32388
b423ec6
4c861e7
6332156
26b6ae4
3a04a2d
b6ce02c
2ddde10
da710a1
4a8607b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| # 로또 - 클린 코드 | ||
|
|
||
| ## 기능 요구사항 | ||
| ### 1단계 | ||
| - [x] 로또 구입 금액을 입력하면 구입 금액에 해당하는 로또를 발급해야 한다. | ||
| - [x] 로또 1장의 가격은 1000원이다. | ||
| - [x] 배열 대신 컬렉션을 사용한다. | ||
| - [x] 줄여 쓰지 않는다(축약 금지). | ||
| - [x] 함수(또는 메서드)의 길이가 10라인을 넘어가지 않도록 구현한다. | ||
| - [x] 함수(또는 메서드)가 한 가지 일만 하도록 최대한 작게 만들어라. | ||
| ### 2단계 | ||
| - [x] 로또 당첨 번호를 받아 일치한 번호 수에 따라 당첨 결과를 보여준다. | ||
| - [x] 모든 원시 값과 문자열을 포장한다. | ||
| - [x] 일급 컬렉션을 쓴다. | ||
|
|
||
| ### 리뷰 반영 | ||
| - [x] 로또 shuffle 사용해보기 | ||
| - [x] MATCHED_TRHEE_COUNT, MATHCED_FOUR_COUNT 오타 수정 | ||
| - [ ] ~~SequenceGenerator 테스트 폴더로 이동~~ | ||
| - [x] winningResult 구조 개선 생각해보기(가독성) | ||
| - [ ] ~~로또 티켓 생성 및 테스트 테스트 전용 빌더/팩토리 적용해보기~~ | ||
|
|
||
| ### 느낀점 | ||
| - 인텔리제이의 오타 수정 경고를 잘 이용하자 | ||
| - 코드가 깔끔하니까 다시 봤을 때 수정하기도 편하다 | ||
| - 셔플을 쓰니, LottoService의 랜덤성 때문에 테스트가 어려워졌다.... 근데 코드는 더 깔끔해진 것 같다... |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| import controller.Controller; | ||
|
|
||
| public class Main { | ||
| public static void main(String[] args) { | ||
| Controller controller = new Controller(); | ||
| controller.run(); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| package controller; | ||
|
|
||
| import domain.LottoService; | ||
| import domain.LottoTicket; | ||
| import domain.Statistic; | ||
| import domain.WinningResult; | ||
| import view.InputView; | ||
| import view.OutputView; | ||
|
|
||
| import java.util.List; | ||
|
|
||
| public class Controller { | ||
| InputView inputView = new InputView(); | ||
| OutputView outputView = new OutputView(); | ||
| LottoService lottoService = new LottoService(); | ||
| Statistic statistic = new Statistic(); | ||
|
|
||
|
|
||
| public void run(){ | ||
| int money = inputView.getMoney(); | ||
|
|
||
| List<LottoTicket> lottoTickets = lottoService.buyTickets(money); | ||
|
|
||
| outputView.printLottoList(lottoTickets.size(), lottoTickets); | ||
|
|
||
| List<Integer> winningNumbers = inputView.getWinningNumbers(); | ||
|
|
||
| WinningResult winningResult = statistic.getWinningResult(lottoTickets, winningNumbers); | ||
| double revenue = statistic.getRevenue(money, winningResult); | ||
|
|
||
| outputView.printResult(winningResult, revenue); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| package domain; | ||
|
|
||
|
|
||
| import java.util.ArrayList; | ||
| import java.util.Collections; | ||
| import java.util.List; | ||
| import java.util.stream.Collectors; | ||
| import java.util.stream.IntStream; | ||
|
|
||
| public class LottoService { | ||
| public static final int TICKET_PRICE = 1000; | ||
| public static final int LOTTO_SIZE = 6; | ||
|
|
||
| public LottoService() { | ||
| } | ||
|
|
||
| public List<LottoTicket> buyTickets(int money) { | ||
| validateMoney(money); | ||
| int ticketCount = money/ TICKET_PRICE; | ||
|
|
||
| List<LottoTicket> tickets = new ArrayList<>(); | ||
|
|
||
| for(int i=0;i<ticketCount;i++){ | ||
| tickets.add(generateTicket()); | ||
| } | ||
|
Comment on lines
+17
to
+25
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 말씀 주신 tickets.get(0).getTicketNumbers() 같은 사용 방식은, tickets가 단순 List로 노출되어 있어서 생기는 문제처럼 느껴졌습니다! 그래서 이 부분은 LottoTickets처럼 한 번 감싸는 방향도 떠올랐는데, 저도 바로 “그럼 모든 컬렉션을 다 일급 컬렉션으로 만들어야 하나?”라는 고민이 이어지더라고요 ㅋㅋ 어떻게 하면 좋을까요?? (저는 이 죽음의 이지선다에 살아남을 수 있는 방법을 만들어두어서 너무 편하더라고요! 이 기회에 승원님도 하나 만들어보시죠!)
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 아아 그쵸!! 저만의 기준을 세워야겠네요... 지금 생각나는 기준은.. 지금 처럼 코드가독성이 별로인 부분은 사용하는 게 좋을 것 같아요!! 요즘 assertj에서 "코드를 이런 식으로 영어원문 처럼 짤 수도 있구나"라고 감탄해서 그렇게 표현 방식을 다르게 해서 얻는 이득이 지금처럼 크다면 바꾸는게 좋은 것 같습니다. 생각할 기회를 주셔서 감사합니다 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ㅎㅎㅎㅎㅎ 화이팅이에요! |
||
|
|
||
| return tickets; | ||
| } | ||
|
|
||
| private LottoTicket generateTicket() { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 로또 자동 구매 힌트에 Collections.shuffle() 활용 이야기가 적혀 있었는데, 사용하지 않으신 건 의도하신 걸까요???? 🙋 (저는 이런 일탈도 좋아해요! ㅋㅋㅋㅋ) 단순히 “안 썼다”가 궁금한 건 아니고, 현재 방식처럼 중복을 검사하면서 뽑는 방식으로 가져가신 의도가 궁금했습니당!
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 실망하실지도 모르겠지만.... 작업을 하면서 리드미에 다 할 일을 옮겨두어서 리드미만 보고 작업해서 힌트를 못 본게 가장 큰 원인이었습니다... ㅋㅋㅋㅋㅋ 음.... 이제 생각해보니 바구니를 섞어서 뽑는다고 생각하면 shuffle이 이치에 맞는 것 같습니다 하하;; There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 마자용 솔직하게 말씀해주셔서 감사합니당 ㅋㅋㅋㅋㅋ (실망은 안해용 👍) |
||
| List<Integer> ticketBasket = new ArrayList<>(); | ||
| List<Integer> ticketNumbers = new ArrayList<>(); | ||
|
|
||
| ticketBasket = IntStream.rangeClosed(1,45) | ||
| .boxed() | ||
| .collect(Collectors.toList()); | ||
| Collections.shuffle(ticketBasket); | ||
| ticketNumbers = ticketBasket.subList(0, 6); | ||
|
|
||
| return new LottoTicket(ticketNumbers); | ||
| } | ||
|
|
||
| private void validateMoney(int money) { | ||
| if(money < TICKET_PRICE) { | ||
| throw new IllegalArgumentException("1000원 이상의 금액을 입력해주세요."); | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| package domain; | ||
|
|
||
| import java.util.Collections; | ||
| import java.util.List; | ||
|
|
||
| public class LottoTicket { | ||
| List<Integer> lottoNumbers; | ||
|
|
||
| public LottoTicket (List<Integer> lottoNumbers) { | ||
| this.lottoNumbers = lottoNumbers; | ||
| Collections.sort(this.lottoNumbers); | ||
| } | ||
|
Comment on lines
+9
to
+12
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. LottoTicket 생성자에서 받은 리스트를 그대로 들고 있고, 정렬도 원본 리스트에 직접 적용하고 있어서 지금 단계에서는 크게 문제 없을 수 있지만,
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 엇 this.lottoNumbers에 해서 객체 내부의 상태를 바꾼다고 생각했는데 아닌가요?!!?! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 하하하하하 저도 잘 모르겠네요! 한번 실험해주시고 알려주세요! ㅎㅎ |
||
|
|
||
| public List<Integer> getTicketNumbers() { | ||
| return this.lottoNumbers; | ||
| } | ||
|
|
||
| public int getMatchedNumbers(List<Integer> winningNumbers) { | ||
| int count = 0; | ||
| for(Integer winningNumber: winningNumbers) { | ||
| if(lottoNumbers.contains(winningNumber)) { | ||
| count++; | ||
| } | ||
| } | ||
| return count; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| package domain; | ||
|
|
||
| import java.util.List; | ||
|
|
||
| public class Statistic { | ||
|
|
||
|
|
||
| public static final int MATCHED_THREE_PRIZE = 5000; | ||
| public static final int MATCHED_FOUR_PRIZE = 50000; | ||
| public static final int MATCHED_FIVE_PRIZE = 1500000; | ||
| public static final int MATCHED_SIX_PRIZE = 2000000000; | ||
| public static final int MATCHED_THREE_COUNT = 3; | ||
| public static final int MATCHED_FOUR_COUNT = 4; | ||
| public static final int MATCHED_FIVE_COUNT = 5; | ||
| public static final int MATCHED_SIX_COUNT = 6; | ||
|
|
||
| public WinningResult getWinningResult(List<LottoTicket> lottoTickets, List<Integer> winningNumbers) { | ||
| WinningResult winningResult = new WinningResult(); | ||
|
|
||
| for(LottoTicket lottoTicket: lottoTickets) { | ||
| int matchedCount = lottoTicket.getMatchedNumbers(winningNumbers); | ||
|
|
||
| winningResult.add(matchedCount); | ||
| } | ||
| return winningResult; | ||
| } | ||
|
|
||
| public double getRevenue(int money, WinningResult winningResult){ | ||
| double prize = MATCHED_THREE_PRIZE * winningResult.getCountOf(MATCHED_THREE_COUNT) | ||
| + MATCHED_FOUR_PRIZE * winningResult.getCountOf(MATCHED_FOUR_COUNT) | ||
| + MATCHED_FIVE_PRIZE * winningResult.getCountOf(MATCHED_FIVE_COUNT) | ||
| + MATCHED_SIX_PRIZE * winningResult.getCountOf(MATCHED_SIX_COUNT); | ||
| return prize/money; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| package domain; | ||
|
|
||
| import java.util.Collections; | ||
| import java.util.List; | ||
|
|
||
| public class WinningResult { | ||
|
|
||
| public static final int WINNING_COUNT = 1; | ||
| public static final int LOTTO_COUNT_BASE = 0; | ||
|
|
||
| List<Integer> winningResult = Collections.nCopies(6, LOTTO_COUNT_BASE); | ||
|
|
||
| public WinningResult() { | ||
| } | ||
|
|
||
| public void add(int matchedCount){ | ||
| winningResult.set(matchedCount, winningResult.get(matchedCount) + WINNING_COUNT); | ||
| } | ||
|
|
||
| public int getCountOf(int MatchedCount){ | ||
| return winningResult.get(MatchedCount); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| package view; | ||
|
|
||
| import java.util.Arrays; | ||
| import java.util.List; | ||
| import java.util.Scanner; | ||
| import java.util.stream.Collectors; | ||
|
|
||
| public class InputView { | ||
| Scanner scanner = new Scanner(System.in); | ||
|
|
||
| public int getMoney() { | ||
| System.out.println("구입금액을 입력해 주세요."); | ||
| int money = scanner.nextInt(); | ||
| scanner.nextLine(); | ||
| return money; | ||
| } | ||
|
|
||
| public List<Integer> getWinningNumbers(){ | ||
| System.out.println("지난 주 당첨 번호를 입력해 주세요."); | ||
| String winningNumbers = scanner.nextLine(); | ||
| return Arrays.stream(winningNumbers.split(",")) | ||
| .map(String::trim) // 공백 제거 | ||
| .map(Integer::parseInt) | ||
| .collect(Collectors.toList()); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| package view; | ||
|
|
||
| import domain.LottoTicket; | ||
| import domain.WinningResult; | ||
|
|
||
| import java.util.List; | ||
|
|
||
| public class OutputView { | ||
|
|
||
| public void printLottoList(int count, List<LottoTicket> lottoTickets){ | ||
| System.out.println( count + "개를 구매했습니다."); | ||
|
|
||
| for (LottoTicket lottoTicket : lottoTickets) { | ||
| System.out.println(lottoTicket.getTicketNumbers().toString()); | ||
| } | ||
| } | ||
|
|
||
| public void printResult(WinningResult winningResult, double revenue) { | ||
| System.out.println("당첨 통계"); | ||
| System.out.println("---------"); | ||
| System.out.println("3개 일치 (5000원)- "+ winningResult.getCountOf(3) +"개"); | ||
| System.out.println("4개 일치 (50000원)- "+ winningResult.getCountOf(4) +"개"); | ||
| System.out.println("5개 일치 (1500000원)- "+ winningResult.getCountOf(5) +"개"); | ||
| System.out.println("6개 일치 (2000000000원)- "+ winningResult.getCountOf(6) +"개"); | ||
| System.out.println("총 수익률은 "+ revenue +"입니다."); | ||
|
|
||
| } | ||
| } |
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.
Controller 안에서 필요한 객체들을 직접 생성하고 있어서, 현재는 동작에는 문제가 없지만 테스트하거나 조합을 바꾸고 싶을 때 조금 덜 유연해질 수 있을 것 같아요!
혹시 해당 방식으로 구현하신 의도가 궁금합니다! 🙋 (수정이 필요하다는 뜻은 아니에용)
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.
컨트롤러가 어플리케이션의 흐름의 관리하는 가장 상위 객체라고 생각해서 만약에 이것들이 바뀌면 문제 자체가 바뀐다고 생각해서 수정을 한다면 컨트롤러를 수정하는 걸 생각했습니다. main에서 컨트롤러가 객체를 받으면 조금 더 유연해질 수도 있겠군요! 만약 컨트롤러에서 사용하는 객체들을 유연하게 바꿀 필요가 있다면 그렇게 설계하는 것도 좋을 것 같네요!! 감사합니다.
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.
ㅎㅎㅎ 인지만 하고 계신다면 나중에 Spring에서 재밌을거에요 🎉