Step4 #1003
Step4 #1003
Conversation
| val autoTickets = lottoShop.provideAutoTickets(ticketCount - manualLottoCount) | ||
| inputView.printPurchasedLotto(autoLottoTickets = autoTickets, manualLottoTickets = manualTickets) | ||
|
|
||
| return LottoTickets(manualTickets.lottoTicketList + autoTickets.lottoTicketList) | ||
| } |
There was a problem hiding this comment.
LottoSimulator내에서 계산하기보단,
LottoShop에게 위임하는 구조는 어덜까요?
manualTickets와 budget 만 부여하면, 내부적으로 알아서 로또를 생성해줄수 있지 않을까요?
(이미 생성된 LottoPurchaseOrder 객체를 활용해서 전달할수도 있을거같아요 )
| override fun create(ticketCount: Int): LottoTickets { | ||
| val lottoTicketList = mutableListOf<LottoTicket>() | ||
| repeat(ticketCount) { | ||
| val lottoTicket = LottoTicket(readln().trim().split(",").map { it.toInt() }.map { LottoNumber(it) }) |
There was a problem hiding this comment.
TicketGenerator는 domain 로직이라고 볼수도 있을거같아요,
생성로직과 View관련 기능은 분리해보면 어떨까요?
| val manualLottoCount = inputView.getManualLottoCount() | ||
|
|
||
| val manualTickets = if (manualLottoCount != 0) { | ||
| inputView.printManualLottoInputString() |
There was a problem hiding this comment.
단순히 print를 실행하기보단,
함수에게 책임을주고, 행위를 위임해보면 어떨까요?
LottoSimulator가 하나씩 print에 관여하기보다, inputView에게 메세지를 전달해보아요!
inputView.getManualNumbers(manualLottoCount)
There was a problem hiding this comment.
이 부분은 view 의 역할과 lotto shop 의 역할을 어디까지로 설정할 지 고민이 좀 되는 부분이네요 ㅎㅎ 일단 가장 적절해 보이는 방식으로 작성해 보겠습니다!
| class ExactTicketGenerator(private val lottoTickets: LottoTickets) : TicketGenerator { | ||
| override fun create(ticketCount: Int): LottoTickets = lottoTickets |
There was a problem hiding this comment.
lottoTickets의 사이즈와 ticketCount 가 일치하지 않는다면 테스트에 문제가 생길수도 있을거같아요
| object ActualLottoShop : LottoShop { | ||
| override fun provideAutoTickets(ticketCount: Int): LottoTickets = AutoTicketGenerator.create(ticketCount) | ||
| override fun provideManualTickets(ticketCount: Int): LottoTickets = ManualTicketGenerator.create(ticketCount) |
There was a problem hiding this comment.
AutoTicketGenerator, ManualTicketGenerator를 싱글턴으로 접근하는구조네요!
싱글턴 패턴은 자주 사용되는 패턴이지만, 잘못사용되면 객체간 결합도를 높여주게 되는 현상이 있어요
| object ActualLottoShop : LottoShop { | |
| override fun provideAutoTickets(ticketCount: Int): LottoTickets = AutoTicketGenerator.create(ticketCount) | |
| override fun provideManualTickets(ticketCount: Int): LottoTickets = ManualTicketGenerator.create(ticketCount) | |
| �class ActualLottoShop( | |
| autoTicketGenerator: TicketGenerator, | |
| manualTicketGenerator: lTicketGenerator | |
| ) : LottoShop { | |
| override fun provideAutoTickets(ticketCount: Int): LottoTickets = autoTicketGenerator.create(ticketCount) | |
| override fun provideManualTickets(ticketCount: Int): LottoTickets = manualTicketGenerator.create(ticketCount) |
위 처럼 바꾼 코드랑은 어떤점이 다를까요? 고민해보시면 좋을거같아요!
| val manualTickets = if (manualLottoCount != 0) { | ||
| lottoShop.provideManualTickets(inputView.getManualNumbers(manualLottoCount)) | ||
| } else LottoTickets(listOf()) | ||
|
|
||
| val autoTickets = lottoShop.provideAutoTickets(ticketCount, manualLottoCount) | ||
| inputView.printPurchasedLotto(autoLottoTickets = autoTickets, manualLottoTickets = manualTickets) |
There was a problem hiding this comment.
자동로또와 수동로또를 구매하는 역할도 LottoShop이 가지고 있어도 좋을거 같다는 생각이들어요!
단순히 provide해주는 객체가 아닌, manualNumbers와 budget 정보만으로 로또를 생성해줄수 있지않을까요?
LottoSimulator는 Controller역할로 View와의 상호작용에 대한 책임만 있지, 어떤 로또가 provide등의 책임은 LottoShop에게 모두 위임해보아요!
|
|
||
| interface LottoShop { | ||
| fun provideAutoTickets(ticketCount: Int, preGeneratedTicketCount: Int): LottoTickets | ||
| fun provideManualTickets(manualNumbersList: List<List<Int>>): LottoTickets |
| private val autoTicketGenerator: AutoTicketGenerator, | ||
| private val manualTicketGenerator: ManualTicketGenerator, |
There was a problem hiding this comment.
AutoTicketGenerator, ManualTicketGenerator
추상화를 제거하고, 하나의 object 클래스로 만들어주셨군요
TicketGenerator라는 추상화를 해보면 어떨까요?
ActualLottoShop 관련해서 Fake객체를 주입시켜, 구체적인 테스트도 작성할수 있을거같아요
| class MockLottoShop(private val manualTicketGenerator: ManualTicketGenerator) : LottoShop { | ||
| override fun provideAutoTickets(ticketCount: Int, preGeneratedTicketCount: Int): LottoTickets = LottoTickets(listOf()) | ||
| override fun provideManualTickets(manualNumbersList: List<List<Int>>): LottoTickets = | ||
| manualTicketGenerator.create(manualNumbersList) |
There was a problem hiding this comment.
해당 Mock객체는 정상적인 동작을 한다고 보기는 힘들지 않을까요?
fake객체라면 원하는대로 활용할수있는구조로 작성해봐도 좋을거같아요
추상화를 한 이유와 Fake객체를 사용하는 이유가 무엇일지 고민해보면 어떨까요?
테스트가능한 구조를 구민보아요
| class MockLottoShop(private val manualTicketGenerator: ManualTicketGenerator) : LottoShop { | |
| override fun provideAutoTickets(ticketCount: Int, preGeneratedTicketCount: Int): LottoTickets = LottoTickets(listOf()) | |
| override fun provideManualTickets(manualNumbersList: List<List<Int>>): LottoTickets = | |
| manualTicketGenerator.create(manualNumbersList) | |
| class MockLottoShop( | |
| private val autoTickets: LottoTickets, | |
| private val manualTicket: LottoTickets, | |
| ) : LottoShop { | |
| override fun provideAutoTickets(ticketCount: Int, preGeneratedTicketCount: Int): LottoTickets = autoTickets | |
| override fun provideManualTickets(manualNumbersList: List<List<Int>>): LottoTickets = manualTicket |
리팩토링
수동 로또 기능 구현