-
Notifications
You must be signed in to change notification settings - Fork 57
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
[로또 미션] 김명지 미션 제출합니다. #34
Open
Starlight258
wants to merge
27
commits into
next-step:starlight258
Choose a base branch
from
Starlight258:feat/step4,5
base: starlight258
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 21 commits
Commits
Show all changes
27 commits
Select commit
Hold shift + click to select a range
5abe2a2
refactor: 리팩토링
Starlight258 685ce55
fix: 우승자 예외 처리 버그 수정
Starlight258 0f84cc2
refactor: 함수형 인터페이스 활용
Starlight258 7def5fb
refactor: 메서드 private으로 변경
Starlight258 6fbbe0b
test: WinningRank 테스트 작성
Starlight258 204eec4
refactor: 반환값 수정
Starlight258 dd3feb1
feat: 로또 수동 구매 기능 구현
Starlight258 3f1a41a
feat: LottoCount 값 객체 생성
Starlight258 e3c8191
refactor: inputView에서 outputView 의존성 제거
Starlight258 f01e1ac
fix: 가격 배수 더하기 버그 수정
Starlight258 2b05742
refactor: 리팩토링
Starlight258 48e7deb
refactor: 메서드 10줄 이하로 리팩토링
Starlight258 0232fab
test: LottoCount 테스트 추가
Starlight258 49c785c
test: 테스트 작성
Starlight258 c61bd49
refactor: 사용하지 않는 메서드 삭제
Starlight258 17e4d69
refactor: Lotto 내부 컬렉션 Set로 변경
Starlight258 dda74be
refactor: 매개변수에 final 사용하여 불변성 표현
Starlight258 6a3581d
rename: LottoResult를 LottoStatistics로 변경
Starlight258 63711c3
refactor: 필요없는 메서드 제거 및 메서드명 변경
Starlight258 c4f33e2
refactor: 입력을 받는 컬렉션에 방어적 복사 수행
Starlight258 20eb06c
docs: README 업데이트
Starlight258 7b9c6fa
refactor: 메서드 파라미터 타입 변경
Starlight258 e715e31
refactor: LottoStatistics에서 WinningRank 자체를 key로 사용
Starlight258 4e486aa
rename: LottoStatistics.java -> LottoResult.java
Starlight258 2ff823c
feat: lotto 출력 오름차순으로 수정
Starlight258 4188aec
rename: 패키지명 변경
Starlight258 2134c60
docs: README.md 업데이트
Starlight258 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,8 @@ | ||
package org.duckstudy.model; | ||
|
||
import java.util.Objects; | ||
import org.duckstudy.model.lotto.LottoResult; | ||
import org.duckstudy.model.lotto.LottoCount; | ||
import org.duckstudy.model.lotto.LottoStatistics; | ||
import org.duckstudy.model.lotto.constant.WinningRank; | ||
|
||
public class Price { | ||
|
@@ -13,16 +14,16 @@ public class Price { | |
|
||
private final int value; | ||
|
||
public Price(int price) { | ||
public Price(final int price) { | ||
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. 파라미터들에 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. 입력값이 객체라면 참조가 변경되는 것을 막고, 기본 값이라면 값 자체가 변경되는 것을 막아서 실수를 방지할 수 있기 때문입니다. |
||
validatePrice(price); | ||
this.value = price; | ||
} | ||
|
||
public static Price initialize() { | ||
public static Price zero() { | ||
return new Price(INCLUSIVE_MIN_PRICE); | ||
} | ||
|
||
private void validatePrice(int price) { | ||
private void validatePrice(final int price) { | ||
if (price < INCLUSIVE_MIN_PRICE) { | ||
throw new IllegalArgumentException(String.format("가격은 %d원 이상이어야 합니다.", INCLUSIVE_MIN_PRICE)); | ||
} | ||
|
@@ -34,41 +35,36 @@ public void validateInputPrice() { | |
} | ||
} | ||
|
||
public Price addPrice(int value) { | ||
public Price addPrice(final int value) { | ||
return new Price(this.value + value); | ||
} | ||
|
||
public Price multiplyTimes(int times) { | ||
return new Price(value * times); | ||
} | ||
|
||
public double divideByPrice(Price divisor) { | ||
public double divideByPrice(final Price divisor) { | ||
checkIfZero(divisor.getValue()); | ||
return (double) value / divisor.getValue(); | ||
} | ||
|
||
private void checkIfZero(int divisor) { | ||
private void checkIfZero(final int divisor) { | ||
if (divisor == ZERO) { | ||
throw new IllegalArgumentException(String.format("%d으로 나눌 수 없습니다.", ZERO)); | ||
} | ||
} | ||
|
||
public int calculateLottoCount() { | ||
public LottoCount calculateLottoCount() { | ||
checkIfZero(LOTTO_PRICE); | ||
return value / LOTTO_PRICE; | ||
return new LottoCount(value / LOTTO_PRICE); | ||
} | ||
|
||
public double calculateProfitRate(LottoResult result) { | ||
Price profit = Price.initialize(); | ||
public double calculateProfitRate(final LottoStatistics result) { | ||
Price profit = Price.zero(); | ||
for (WinningRank winningRank : WinningRank.values()) { | ||
profit = profit.accumulateProfit(winningRank, result.getMatchingCount(winningRank.getKey())); | ||
} | ||
return profit.divideByPrice(this) * PERCENT_BASE; | ||
} | ||
|
||
private Price accumulateProfit(WinningRank winningRank, int count) { | ||
return this.addPrice(winningRank.getPrice()) | ||
.multiplyTimes(count); | ||
private Price accumulateProfit(final WinningRank winningRank, final int count) { | ||
return this.addPrice(winningRank.getPrice() * count); | ||
} | ||
|
||
public int getValue() { | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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가 아니며, MVC에서의 Conroller와도 달라요.
일단 MVC에서의 Controller를 먼저 이야기하면 Controller는 View를 들고 실제로 랜더링하는 역할은 아니에요. 단순한 콘솔게임이기 때문에 MVC 패턴의 복잡한 정의를 사용하여 구현할 필요는 없어요.
그래서 MVC 패턴을 제거하고보면 Controller라는 네이밍은 클래스와는 어울리지 않아요. 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.
그렇네요..!
Controller
라고 하기엔 잘못된 입력을 반복적으로 받는 로직도 있고, 비즈니스 로직도 호출하고 여러가지를 하고 있네요.MVC 패턴으로 먼저 구조를 잡고 개발하다보니 콘솔 애플리케이션 구현에는 적절하지 않은 설계가 된 것 같아요.
앞으로도 너무 틀에 갇혀서 개발하지 않고 시야를 넓히는 식으로 공부하겠습니다. 감사합니다 !! 😃