Step4 - 로또(수동) #997
Step4 - 로또(수동) #997
Conversation
| val correctCount = winningLotto.lotto.numbers.containsCount(numbers) | ||
| val matchBonus = numbers.contains(winningLotto.bonusNumber) |
There was a problem hiding this comment.
getter를 없애는 방법은 디미터의 법칙에 대해서 읽어보시면 좋을 것 같네요.
간단하게는 값을 꺼내서 로직을 구현하는 대신 책임이 있는 객체에게 메세지를 보내는 방식이라고 생각하시면 될 것 같아요.
| val manualLottosPrice = boughtManualLottoCounts * LottoStore.LOTTO_PRICE | ||
|
|
||
| val checkedLottos = boughtLottos.matchAll(winningLotto) | ||
| val returnRate = checkedLottos.totalReward() / money.toDouble() | ||
| require(money >= manualLottosPrice) { | ||
| "로또를 구매할 돈이 부족합니다." |
There was a problem hiding this comment.
돈에 관한 로직들이 생겨나고 있는 것 같은데요 돈에 관련된 값 객체 등을 만들어서 도메인에 로직을 두면 좋을 것 같아요.
지금은 컨트롤러에 로직들이 있어서 테스트가 어려워지는 것 같습니다.
| fun buyLotto(lottoNumbersGenerateStrategy: LottoNumbersGenerateStrategy): Lotto { | ||
| return lottoGenerator.publish(lottoNumbersGenerateStrategy) | ||
| } | ||
|
|
||
| fun buyLottos(money: Int, lottoNumbersGenerateStrategy: LottoNumbersGenerateStrategy): Lottos { |
There was a problem hiding this comment.
buyLotto는 수동 로또만, butLottos는 자동 로또만 만들고 있습니다.
전략을 주입받는 이유가 있을까요?
| return value.map { it.value } | ||
| .zipWithNext { a, b -> a <= b } |
There was a problem hiding this comment.
LottoNumber가 Comparable을 구현하면 값을 꺼내지 않고도 구현할 수 있을 것 같습니다.
| val numbers: List<Int> | ||
| get() = value.map { it.value } |
There was a problem hiding this comment.
저는 값 객체로 감싼 값들은 어플리케이션의 종단에서만 꺼내서 사용하려고 하는 편입니다. 예를 들면 View에 값을 넣어주기 직전이나 DB 등에 값을 넣기 직전이 되겠죠.
원시값을 객체로 감쌀 때는 그 이유가 있을 거에요. 타입 구분을 위함이라던지 값에 로직을 넣는다던지요. 그런데 어플리케이션 중간에 그 값을 꺼내서 사용해버리면 이렇게 감싼 이유가 사라지게됩니다. LottoNumbers의 클라이언트 입장에서는 이 값이 로또 번호라는 보장을 잃게 되겠죠.
| require(value.size == Lotto.NUMBERS_COUNT) { | ||
| "로또 번호는 항상 ${Lotto.NUMBERS_COUNT}개 여야 합니다." |
| fun showBoughtLottos(lottos: Lottos) { | ||
| println("${lottos.value.size}개를 구매했습니다.") | ||
| fun showBoughtLottos(lottos: Lottos, manualLottoCount: Int) { | ||
| println("수동으로 ${manualLottoCount}장 자동으로 ${lottos.value.size - manualLottoCount}개를 구매했습니다.") |
There was a problem hiding this comment.
굳이 따져보자면 이런 빼기도 로직으로 볼 수 있을 것 같네요.
간단한 어플리케이션이니만큼 최대한 도메인 로직은 도메인에서 처리해보면 좋을 것 같아요.
안녕하세요! step4 구현하여 리뷰요청 드립니다 🙇
전반적으로 로직을 적절한 위치로 옮기는데에 초점을 맞춰 구현하였습니다.
작업하면서 테스트 통과여부만 신경쓰고 잘 신경을 못써서 커밋이 좀 분산되어버렸습니다.. 😓
#953 (comment) 에서 남겨주신 코멘트도 반영하고자 했는데
어떤 방식으로 뎁스를 줄이는게 좋을지 모르겠어서 남겨둔채로 올립니다!