-
Notifications
You must be signed in to change notification settings - Fork 107
[SCG] 최연우 로또 1-2단계 미션 제출합니다 #160
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
a3b8187
993062d
3d57fd3
3399121
8735a45
af85ed0
fb71049
353eabb
ba27609
87bd315
d96f57a
e143b1b
5ff3e9a
c7b89f5
037d4d5
72d1a26
9ef612d
cef7a09
d358d09
8ef2d2a
25f1ea8
3750790
e6e5067
856a743
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,32 @@ | ||
| # 1단계 기능 요구사항 | ||
|
|
||
| - [X] 로또 구입 금액을 입력하면 구입 금액에 해당하는 로또를 발급해야 한다. | ||
| - [X] 로또 1장의 가격은 1000원이다. | ||
|
|
||
| ## 새로운 프로그래밍 요구사항 | ||
|
|
||
| - [X] 배열 대신 컬렉션을 사용한다. | ||
| - [X] 줄여 쓰지 않는다(축약 금지). | ||
| - [X] 함수(또는 메서드)의 길이가 10라인을 넘어가지 않도록 구현한다. | ||
| - [X] 함수(또는 메서드)가 한 가지 일만 하도록 최대한 작게 만들어라. | ||
|
|
||
| # 2단계 기능 요구사항 | ||
|
|
||
| - [X] 로또 당첨 번호를 받아 일치한 번호 수에 따라 당첨 결과를 보여준다. | ||
|
|
||
| ## 새로운 프로그래밍 요구사항 | ||
|
|
||
| - [X] 모든 원시 값과 문자열을 포장한다. | ||
| - [X] 일급 컬렉션을 쓴다. | ||
|
|
||
| # 기존 프로그래밍 요구사항 | ||
|
|
||
| 자바 코드 컨벤션을 지키면서 프로그래밍한다. | ||
| 기본적으로 Java Style Guide을 원칙으로 한다. | ||
| indent(인덴트, 들여쓰기) depth를 2를 넘지 않도록 구현한다. 1까지만 허용한다. | ||
| 예를 들어 while문 안에 if문이 있으면 들여쓰기는 2이다. | ||
| 힌트: indent(인덴트, 들여쓰기) depth를 줄이는 좋은 방법은 함수(또는 메서드)를 분리하면 된다. | ||
| 3항 연산자를 쓰지 않는다. | ||
| else 예약어를 쓰지 않는다. | ||
| else 예약어를 쓰지 말라고 하니 switch/case로 구현하는 경우가 있는데 switch/case도 허용하지 않는다. | ||
| 힌트: if문에서 값을 반환하는 방식으로 구현하면 else 예약어를 사용하지 않아도 된다. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| #Mon Mar 04 13:53:10 KST 2024 | ||
| distributionBase=GRADLE_USER_HOME | ||
| distributionPath=wrapper/dists | ||
| distributionUrl=https\://services.gradle.org/distributions/gradle-8.4-bin.zip | ||
| distributionUrl=https\://services.gradle.org/distributions/gradle-8.14.4-bin.zip | ||
| networkTimeout=10000 | ||
| validateDistributionUrl=true | ||
| zipStoreBase=GRADLE_USER_HOME | ||
| zipStorePath=wrapper/dists |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| import domain.Cashier; | ||
| import domain.Lotto; | ||
| import domain.LottoResult; | ||
| import domain.LottoTicket; | ||
| import domain.LottoTicketGenerator; | ||
| import domain.Price; | ||
| import domain.RandomLottoTicketGenerator; | ||
| import view.InputView; | ||
| import view.OutputView; | ||
|
|
||
| public class Application { | ||
| public static void main(String[] args) { | ||
| InputView inputView = new InputView(); | ||
| OutputView outputView = new OutputView(); | ||
| LottoTicketGenerator lottoTicketGenerator = new RandomLottoTicketGenerator(); | ||
| Cashier cashier = new Cashier(lottoTicketGenerator); | ||
|
|
||
| Price price = inputView.inputPrice(); | ||
| Lotto lotto = cashier.generateTickets(price); | ||
| outputView.showLottoTickets(lotto); | ||
|
|
||
| LottoTicket winnerTicket = inputView.getWinnerTicket(); | ||
| LottoResult result = lotto.getResults(winnerTicket); | ||
| outputView.showLottoResults(result, cashier.getProfitRate(result, price)); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| package domain; | ||
|
|
||
| import java.util.ArrayList; | ||
| import java.util.List; | ||
|
|
||
| public class Cashier { | ||
| private final LottoTicketGenerator lottoTicketGenerator; | ||
|
|
||
| public Cashier(LottoTicketGenerator lottoTicketGenerator) { | ||
| this.lottoTicketGenerator = lottoTicketGenerator; | ||
| } | ||
|
|
||
| public Lotto generateTickets(Price price) { | ||
| Count ticketCount = price.getBuyableLottoCount(); | ||
| List<LottoTicket> generatedTickets = new ArrayList<>(); | ||
| for (int i = 0; i < ticketCount.count(); i++) { | ||
| generatedTickets.add(lottoTicketGenerator.generate()); | ||
| } | ||
| return new Lotto(generatedTickets); | ||
| } | ||
|
|
||
| public double getProfitRate(LottoResult result, Price price) { | ||
| int totalProfit = 0; | ||
| for (LottoRank lottoRank : LottoRank.values()) { | ||
| totalProfit += lottoRank.getPrizeMoney() * result.getMatchCount(lottoRank).count(); | ||
| } | ||
| return price.calculateProfitRate(totalProfit); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| package domain; | ||
|
|
||
| public record Count(int count) { | ||
| public Count { | ||
| validate(count); | ||
| } | ||
|
|
||
| private void validate(int count) { | ||
| if (count < 0) { | ||
| throw new IllegalArgumentException("개수가 음수일 수 없습니다."); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public String toString() { | ||
| return String.valueOf(count); | ||
| } | ||
| } | ||
|
Comment on lines
+3
to
+18
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.
Count의 역할에 대해 잘 고민해주셨어요!😊 다만 getter를 쓰는 이유가 "역산 가능하니 숨겨도 무의미하다"라면, getter를 쓰는 방향은 맞아요. 그리고 한 가지 더 고민해볼 부분이 있어요. 앞선 리뷰처럼 값을 객체로 포장하는 것은 도메인 개념을 표현할 때 의미있는데, "타입 안전성을 위한 래퍼"로서의 가치는 있지만, 현재 Count가 쓰이는 맥락들은 다음과 같아요.
맥락적으로 보았을 때, Count는 int로 대체되어도 표현에서 크게 차이점이 없다는 점에서 눈에 들어왔어요. 그로 인해, 무엇을 위한 Count인지, 예상할 수 없었어요.🤔
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. 이 값은 양수여야 한다는 것을 알려주고 싶었는데, 굳이 필요 없을 수도 있겠네요. |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| package domain; | ||
|
|
||
| import java.util.ArrayList; | ||
| import java.util.Collections; | ||
| import java.util.List; | ||
|
|
||
| public class Lotto { | ||
| private final List<LottoTicket> tickets; | ||
|
|
||
| public Lotto(List<LottoTicket> tickets) { | ||
| this.tickets = tickets; | ||
| } | ||
|
|
||
| public LottoResult getResults(LottoTicket winnerTicket) { | ||
| List<Count> matchCountOfEachTicket = new ArrayList<>(); | ||
| for (LottoTicket ticket : tickets) { | ||
| matchCountOfEachTicket.add(ticket.getMatchCount(winnerTicket)); | ||
| } | ||
| List<Count> matchingTicketCounts = new ArrayList<>(); | ||
| for (LottoRank lottoRank: LottoRank.values()) { | ||
| matchingTicketCounts.add(new Count(Collections.frequency(matchCountOfEachTicket, lottoRank.getMatchingNumberCount()))); | ||
| } | ||
| return new LottoResult(matchingTicketCounts); | ||
| } | ||
|
|
||
| public int getNumberOfTickets() { | ||
| return tickets.size(); | ||
| } | ||
|
|
||
| @Override | ||
| public String toString() { | ||
| return String.join("\n", tickets.stream().map(LottoTicket::toString).toList()); | ||
| } | ||
| } |
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.
지난 미션의 랜덤값 테스트를 반영하여
NumberListGenerator를 인터페이스 구현체로 두신 것 같아요!🙌하지만 로또 추첨 전체 과정을 테스트한다면, 현재 인터페이스 구현체를 활용해 출력값을 조절할 수 있을까요?
Uh oh!
There was an error while loading. Please reload this page.
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.
요구사항에 없더라도, 가능하면 테스트 코드도 함께 작성해보면 학습에 좋을 것 같아요~😄
지금처럼 코드 리펙터링이 왕왕 일어날 때,
하나의 코드를 수정할을 때, 어디에 그리고 몇 개의 테스트가 깨지는 지로
얼마나 OOP를 만족하는 중인지 셀프 피드백이 가능합니다!🤔
혹은 테스트 짜기 어렵다면, 그것 또한 잘못된 코드란 뜻일 수도 있구요..!
LottoTicket, Lotto, Cashier 간 역할 분배에 대한 고민 해결에도 도움이 될 거에요!😄
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.
테스트 작성했습니다!
사실, 이전 미션에서 저는 interface를 사용하지 않고 테스트 케이스들을 직접 작성했습니다.
조금 이해가 안되는 것이, 클래스마다 데이터 타입이 상당히 다르고 테스트 내용도 다르기 때문에 하나의 generator를 반복적으로 사용할 수 없어서, 그냥 각 테스트에서 값을 작성하는 것이 더 낫다고 느꼈습니다.
또한, 테스트에서 어떤 값이 사용되었는지 보기 위해 다른 파일로 이동해야 하는 점도 불편할 것 같았습니다.
반복적인 값으로 여러 클래스를 테스트할 수 있는 경우가 아니면 고정된 NumberGenerator는 불필요하다고 느낍니다.
이에 대해 의견 부탁드립니다!
또한, TICKET_LENGTH와 같은 여러 클래스에 걸쳐 사용되는 값들을 Constants라던가 하는 클래스에 담는 아이디어를 생각했는데, (공개하면 안되는 값이 아니라면) 괜찮을 지 궁금합니다!
또한, priceOfOneLotto처럼 한 클래스에서 사용되는 상수도, 변경하게 되면 Test코드에는 반영이 안되는 문제점에 대해, 그 상수를 public으로 만들고 Test에서 사용하는 것은 어떤지 궁금합니다. 아니면 test 클래스에서 똑같은 상수를 선언해 두는 것은 어떤지 궁금합니다!
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.
Constants는 응집도가 떨어져서 안티패턴으로 취급되기도 합니다!
이 상수들은 서로 연관성이 없습니다.
단지
static final이라는 이유만으로 한 곳에 모이게 됩니다. 애매하면 결국 여기에 몰아넣는 경우도 생기구요..!😮이러면, 티켓 규칙이 바뀌어도, 가격이 바뀌어도, 에러 메시지가 바뀌어도 Constants를 함께 수정해야 합니다.
이는 SRP 위반이구요.🤔
또한, TICKET_LENGTH가 누구 개념인지 Constants만 보면 알 수 없습니다. LottoTicket에 있어야 해당 상수가 누구 소유인지 명확해집니다.
이런 측면에서는 Constants가 필요해 보이는 순간, 사실 그 상수의 주인을 정의하지 못했을 가능성도 있구요.🤔
따라서
public static final int ARRAY_INIT_SIZE = 10;와 같이 구현 세부 사항을 나타내는 경우가 아닌,public static final int TICKET_LENGTH = 6;와 같이 비즈니스 로직을 나타내는 경우라면public으로 두어도 좋다고 생각합니다.😄
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.
이 부분에선 코드와 비교했을때, 질문의 상황이 잘 그려지지 않네요..😂
정확히는 인터페이스를 어떻게 활용하셨던 것 인지가 궁금합니다.🤔
interface를 사용한 테스트의 목적은
랜덤값을 고정값으로 바꾸기 위함입니다.때문에 직접적인 랜덤값의 영향을 받는 generateTickets(), 그리고 이를 포함한 전체 테스트에서 검증을 위해 사용됩니다.
이는
generateTickets()안에 있는generatedTickets.add(lottoTicketGenerator.generate());를고정값으로 바꿈으로서, 그 외의 로직이 잘 작동하는 지 확인하는 것입니다.
때문에,
LottoTicketGenerator의 구현체를 바꿔줌으로서, lottoTicketGenerator.generate()가 고정된 값을 출력하도록 하는 것입니다.이때, 다음과 같이 구현체를 만들어볼 수 있겠네요.
이러면 캐서 테스트에서 선언될 때, 필요한 로또 티켓을 넣은 구현체를 통해 랜덤 부분을 고정값으로 바꿀 수 있을 것 같아요.
지난 미션의 랜덤값 테스트를 반영하여 NumberListGenerator를 인터페이스 구현체로 두신 것 같아요!🙌이 부분을 해결하기 위한 방법 중 하나로, 위와 같이 접근해볼 수 있을 것 같아요.
혹시 인터페이스를 어떻게 활용하시고자 했는지 조금 더 설명해주실수있나요?