-
Notifications
You must be signed in to change notification settings - Fork 467
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
[2단계 - 블랙잭 베팅] 우가(민보욱) 미션 제출합니다 #894
Open
bowook
wants to merge
38
commits into
woowacourse:bowook
Choose a base branch
from
bowook:step2
base: bowook
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
Conversation
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
…을 통해 블랙잭인지 확인하기 위함"
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
체크 리스트
test
를 실행했을 때, 모든 테스트가 정상적으로 통과했나요?객체지향 생활체조 요구사항을 얼마나 잘 충족했다고 생각하시나요?
1~5점 중에서 선택해주세요.
선택한 점수의 이유를 적어주세요.
ScoreBaord 객체를 작게 유지하지 못했습니다.
어떤 부분에 집중하여 리뷰해야 할까요?
안녕하세요 베루스! 😄
Step2도 잘 부탁드립니다!
고민 사항 1
문제:
ScoreBoard
가 책임이 너무 많아서 승/패를 판단하는 부분을BattleResultCalculator
에게 책임을 넘겼습니다.이때,
BattleResultCalculator
를determineOutcome
메서드에서 생성하는 것이 적절할지, 아니면 외부에서 생성자를 주입받아서 사용하는 것이 적절할지 고민입니다.고민 1:
BattleResultCalculator
를determineOutcome
메서드에서 생성하면,ScoreBoard
와BattleResultCalculator
간의 결합도가 높아지는 것이 아닌가 하는 생각이 들었습니다.이유는,
BattleResultCalculator
가 현재는 단순히calculate
로 계산을 하는 로직밖에 없지만, 추후에 다른 방식의 계산 방법이 생겨서 바꾸어야 한다면ScoreBoard
까지 영향을 받게 되기 때문입니다.또한, 테스트 코드 작성하기도 불편하다고 생각했습니다.
고민 2:
외부에서 생성자를 주입받아서 사용하는 방법은 결합도를 낮출 수 있고, 테스트 코드도 작성하기 수월하다고 생각했습니다.
그런데,
BattleResultCalculator
는ScoreBoard
의determineOutcome
메서드에서만 사용하는데, 굳이 외부에서 주입 받은 후에 필드에 가지고 있을 필요가 있을까? 라는 생각이 들어 고민 중입니다.고민 사항 2
문제:
"개발자가 반드시 정복해야 할 객체 지향과 디자인 패턴"이라는 책을 읽던 중, 챕터 7장의 2번째에 있는 전략 패턴을 읽게되었습니다.
수익을 계산하는 부분에 전략 패턴을 적용하면,
GamblingStatement
는 외부에서 주입 받은 (무엇인지 모르는) 전략을 가지고 계산된 돈을 이용할 수 있겠다는 생각을 했습니다.그런데, 이 과정에서 고민이 생겼습니다.
고민 1:
책에서 보았던 예시에는 파라미터로 전략을 주입시켜주었습니다.
하지만, 현재
GamblingStatement
는 참가자 각각에 해당하는 계산 전략을 가져와야 했고, 외부에서 각각에 대한 전략을 주입시키기는 힘들지 않을까? 라는 생각이 들었습니다.그래서,
ScoreBoard
를 파라미터로 전달받아 스코어 보드에 계산 전략을 찾도록 하는 메서드를 추가했습니다.이렇게 하면,
GamblingStatement
는 해당 전략이 어떤 계산 전략인지 알 수 없게 됩니다.하지만, 동시에
GamblingStatement
는ScoreBoard
를 알 필요가 없는 것이 아닌가? 하는 생각이 들었습니다.왜냐하면, 전략만 주입받으면 되지 굳이
ScoreBoard
를 알아야 하나? 라는 생각이 들어서 이 부분을 어떻게 개선해야 할지 고민입니다.참가자 각각에 해당하는 전략을
ScoreBoard
에 둔 이유는,ScoreBoard
가 참가자 각각에 해당하는 승/패 정보를 가지고 있기 때문입니다.이렇게 사용해도 문제가 없는 것인지 잘 모르겠습니다.