Step5, 자동차 경주 (리팩터링) #1748
Step5, 자동차 경주 (리팩터링) #1748
Conversation
| val inputView: InputView = ConsoleInputViewFactory.newInstance() | ||
| val resultView: ResultView = ConsoleResultViewFactory.newInstance() | ||
| val numberGenerator: NumberGenerator = RandomGeneratorFactory.newInstance() | ||
| val racingController = RacingControllerFactory.newInstance(inputView, resultView, numberGenerator) | ||
| val racingController = RacingControllerFactory.newInstance(inputView, resultView) |
There was a problem hiding this comment.
타입을 생략한 코드와 그렇지 않은 코드가 섞여있는 것 같아요! 코틀린은 타입추론을 지원하기 때문에 타입을 생략하는 연습을 해보면 어떨까요?
| @@ -1,30 +1,30 @@ | |||
| package racing.view.input | |||
|
|
|||
| internal class ConsoleInputView : InputView { | |||
| override fun displayCarNamesQuestion() { | |||
| private fun displayCarNamesQuestion() { | |||
There was a problem hiding this comment.
개인적으로 코드의 흐름은 위에서 아래로 읽을때 제일 좋은 것 같아요!
그렇다면 public 함수를 위에 작성하고 그 public함수가 호출하는 private함수는 밑에다 두면어떨까요?
| engineFactory = EngineFactory, | ||
| ) | ||
|
|
||
| fun createCars( |
There was a problem hiding this comment.
해당 함수에 대한 테스트가 작성되지 않은 것 같아요!
테스트를 작성해보면 어떨까요?
| import racing.view.input.ConsoleInputView.Companion.DEFAULT_NAME_MAX | ||
| import racing.view.input.ConsoleInputView.Companion.DELIMITER_COMMA | ||
|
|
||
| class MockInputViewImpl( |
There was a problem hiding this comment.
Mock객체를 테스트하는게 유의미할지 고민해보면 좋을 것 같아요!
실제로 운영에서는 쓰이지 않는 코드이니까요 :)
| startRound(cars) | ||
| resultView.displayCarMovement(cars) | ||
| with(gameContext) { | ||
| val carNames = inputView.promptAndValidateCarNamesInput() |
There was a problem hiding this comment.
inputView, prompt라는게 중요한 기능일까요? 핵심적인건 carNames를 입력받아서 전달해준다는 것 같아요!! (prompt와 validate는 부가적인 내용으로 보입니다 :))
| object CarFactory { | ||
| fun createCars( | ||
| carNames: List<String>, | ||
| engineProvider: () -> Engine, |
| constructor(engine: Engine) : this("", engine) | ||
| constructor(name: String) : this(name, DEFAULT_ENGINE) |
There was a problem hiding this comment.
이 두가지 생성자는 어떠한 이유로 필요할까요??
이름이 공백인 자동차나, DEFAULT_ENGINE을 가진 자동차가 필요한 이유가 무엇인지 궁금합니다 :)
| ) { | ||
| val number = numberGenerator.generator() | ||
| if (number >= forwardLimit) { | ||
| fun move(forwardLimit: Int = DEFAULT_FORWARD_LIMIT) { |
There was a problem hiding this comment.
민수님은 주로 default값은 어떨 때 사용하시나요? 여기에도 default는 없어도 될 것 같아보여요!!
| cars.forEachIndexed { index, car -> | ||
| car.move(forwardLimit) | ||
|
|
||
| val hasRoundEnded = cars.size.minus(1) <= index | ||
| onRoundProgress(car, hasRoundEnded) | ||
| } |
There was a problem hiding this comment.
한 라운드 내에서 모든 자동차들의 움직이 끝난다음에 출력을 한번만 해주게끔 변경해주면 어떨까요?
지금은 자동차가 끝날때마다 해주다보니 hasRoundEnded같은 인자도 필요하게 된 것 같아져보여요!
step4 에서 요구하신 사항도 반영하여, step5 과제 진행하였습니다.
PR 리뷰 잘부탁드립니다. 🙏