Skip to content
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

[Step5]자동차 경주(리팩터링) #936

Open
wants to merge 15 commits into
base: choi-ys
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions docs/SETP3-README.md → docs/STEP3-README.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,11 @@
* [x] Service 계층에 위치한 난수 생성부 분리
* Service 계층에 위치한 난수 생성부로 인해 해당 계층이 테스트 하기 어려운 문제를 개선하기 위해 난수 생성부를 Service 계층 분리 후, 외부에서 생성된 난수를 각 `Car`객체에 주입하여 해당 계층을 테스트 가능하도록 수정
* 난수 생성부 위치 변경
* AS*IS : `Car` 객체에서 경주 진행 메서드 호출 시, 난수를 생성
* TO*BE : Car 객체 생성 시, 진행 라운드 만큼 난수 생성 후 Car 객체 필드에 주입
* AS-IS : `Car` 객체에서 경주 진행 메서드 호출 시, 난수를 생성
* TO-BE : Car 객체 생성 시, 진행 라운드 만큼 난수 생성 후 Car 객체 필드에 주입
* 각 차량의 자동차 경주 진행 방식 변경
* AS*IS : 각 차량에서 현재 라운드 진행을 위해 난수 생성 및 생성된 난수에 따른 진행거리 누적 여부 판별
* TO*BE : 외부에서 각 차량에 진행 라운드 만큼 아미 생성된 난수를 판별하여 진행 거리 누적 여부만 판별
* AS-IS : 각 차량에서 현재 라운드 진행을 위해 난수 생성 및 생성된 난수에 따른 진행거리 누적 여부 판별
* TO-BE : 외부에서 각 차량에 진행 라운드 만큼 아미 생성된 난수를 판별하여 진행 거리 누적 여부만 판별
* 로직 변경으로 인해 깨지는 TC 수정
* 생성된 난수 목록 일급 컬렉션 정의
* 진행 라운드 만큼 생성된 난수 목록이 주입되는 일급 컬렉션
Expand Down
26 changes: 26 additions & 0 deletions docs/STEP4-README.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,29 @@
* 우승자 도메인 정의
* 경주 완료 후 자동차 객체 목록을 통해 우승자 객체 생성 및 우승자 목록 추출
* 경주를 완료한 자동차 객체 목록을 통해 우승한 차량의 이름 추출 TC 작성

## 코드 리뷰 피드백 내용 정리
* [x] 별도의 역할 없이 객체를 포장하고 있는 `Cars`일급 컬렉션과 참가한 자동차 목록으로 부터 우승자를 산출하는 `Winners` 일급 컬렉션 역할을 병합
* [ ] 각 라운드 진행 방식 변경
* AS-IS : `Car` 객체에 미리 생성된 난수를 판별하여 주행거리를 누적
* TO-BE : 매 라운드 마다 발생된 난수를 판별한 후, 차량 주행거리를 누적
* [x] 추상화를 통해 난수에 대한 의존성으로 테스트 하기 어려운 현상 개선
* [x] 난수 발생 부를 추상화하여 운영 환경에 주입되는 구현체와 테스트 환경에 주입되는 구현체를 별도로 구현한 뒤, 각 실행 환경에 맞는 구현체를 선택하여 주입함으로써 난수로 인해 테스트 하기 어려운 문제 개선
* 추상화된 난수 생성부 주입을 통한 자동차 경주 진행
* [x] 자동차 경주를 진행하는 `RacingService` 객체에 추상화된 난수 생성 부의 구현체를 주입하여 생성한 후, 각 라운드 진행 시 운영 환경에서는 실제 난수 생성 구현체가 동작하고, 테스트 환경에서는 난수 발생 시 의도한 수를 반환하는 구현체가 동작할 수 있도록 수정
* [x] 난수 생성 부 추상화에 따른 `Car`객체에 자동차를 표현하기 어색한 `RandomNumbers` 필드 제거
* [ ] 결과 출력 시 특정 라운드에 차량에 발생한 난수와, 그 결과를 기록하는 별도의 객체를 이용하도록 수정
* 외부에서 해당 객체에 값에 접근하는 처리 방식에서 객체에 메시지를 보내 그 결과값을 반환받는 방식으로 수정
* 테스트 표현 개선
* [x] 도메인 로직을 활용한 테스트 결과 검증 부를 하드코딩된 값으로 변경
* 테스트 검증 부가 도메인 로직을 참조하고 있음에 따라 로직 변경 시 테스트로서의 역할을 제대로 수행할 수 없는 문제점 개선을 위해 테스트 검증 부를 하드코딩된 값으로 변경
* AS-IS :
```kotlin
given.winnerNames() shouldBe "${두_번째_차량.name}, ${세_번째_차량.name}"
```
* TO-BE :
```kotlin
given.winnerNames() shouldBe "두 번째 차량, 세 번째 차량"
```
* [x] (테스트 픽스처 활용으로 인해 테스트 코드가 결합되는 현상 개선)[https://jojoldu.tistory.com/611]
* 테스트 픽스처로 인해 테스트 코드가 결합되어 테스트 코드 유지보수가 복잡해지는 현상 개선을 위해, 각 테스트에 필요한 fixture 생성부를 private 메서드로 구성
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,17 @@ import step3.racingcar.domain.Cars
import step3.racingcar.domain.PlayInfo
import step3.racingcar.service.RacingCarService
import step3.racingcar.utils.CarGenerator
import step3.racingcar.utils.RandomNumberGenerator.generateRandomNumberToCarByRound
import step3.racingcar.utils.RandomNumberGenerator
import step3.racingcar.view.InputView.Companion.inputJoinerCarsGuideMessagePrinter
import step3.racingcar.view.InputView.Companion.inputRoundCountGuideMessagePrinter

class RacingCarController {
private val racingCarService: RacingCarService = RacingCarService()
private val racingCarService: RacingCarService = RacingCarService(RandomNumberGenerator())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Service 하위의 모든 객체에서 이제 생성되는 숫자에 대해 개발자가 제어할 수 있겠네요 :) 👍👍


fun gameStart() {
val carNames = inputJoinerCarsGuideMessagePrinter()
val totalRound = inputRoundCountGuideMessagePrinter()
val cars = Cars.of(CarGenerator.generate(carNames))
generateRandomNumberToCarByRound(cars, totalRound)
val playInfo = PlayInfo(cars, totalRound)
racingCarService.play(playInfo)
}
Expand Down
10 changes: 3 additions & 7 deletions src/main/kotlin/step3/racingcar/domain/Car.kt
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,10 @@ private const val CAR_ID_DELIMITER = "-"
private const val MOVE_CRITERIA = 4

class Car(val name: String) {
private var randomNumbers: RandomNumbers = RandomNumbers()
var distance = 0
var distance: Int = 0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

distance는 외부에서 접근해서 값을 변경할 수 있겠네요!
외부에서 변경하지 못하게 막아보는 것은 어떨까요?


fun addRandomNumber(randomNumber: Int) = randomNumbers.add(randomNumber)

fun race(currentRoundIndex: Int) {
val randomNumberByCurrentRound: Int = randomNumbers[currentRoundIndex]
if (isMove(randomNumberByCurrentRound)) {
fun race(randomNumber: Int) {
if (isMove(randomNumber)) {
distance++
}
}
Expand Down
17 changes: 15 additions & 2 deletions src/main/kotlin/step3/racingcar/domain/Cars.kt
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,23 @@ package step3.racingcar.domain

class Cars private constructor(private val elements: List<Car>) {
fun elements(): List<Car> = elements
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

elements 프로퍼티를 공개하는 것과 element() 메서드를 따로 만드는 것은 큰 차이가 없어보이네요 :)


fun size(): Int = elements.size
operator fun get(index: Int) = elements[index]

operator fun get(index: Int): Car = elements[index]

fun winnerNames(): String =
findWinnerNames().joinToString(WINNER_NAME_JOINING_SEPARATOR)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

승자들의 이름들을 , 로 묶어내는 일은 뷰의 일로 보이네요 :)


private fun findWinnerNames(): List<String> =
elements()
.filter { it.distance == findMaxDistance() }
.map { it.name }

private fun findMaxDistance(): Int = elements().maxOf { it.distance }

companion object {
fun of(elements: List<Car>) = Cars(elements)
private const val WINNER_NAME_JOINING_SEPARATOR = ", "
fun of(elements: List<Car>): Cars = Cars(elements)
}
}
10 changes: 10 additions & 0 deletions src/main/kotlin/step3/racingcar/domain/RandomNumber.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package step3.racingcar.domain

interface RandomNumber {
fun value(): Int
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

하위 구현체가 "랜덤 숫자 생성기" 이니, 상위 인터페이스는, "숫자 생성기" 정도가 어떨까요?
그러면 다른 구현체를 만들 때, 다른 종류의 숫자 생성기를 만들어볼 수도 있겠네요 🙂


companion object {
const val RANGE_START = 1
const val RANGE_END = 9
}
}
7 changes: 0 additions & 7 deletions src/main/kotlin/step3/racingcar/domain/RandomNumbers.kt

This file was deleted.

22 changes: 0 additions & 22 deletions src/main/kotlin/step3/racingcar/domain/Winners.kt

This file was deleted.

17 changes: 12 additions & 5 deletions src/main/kotlin/step3/racingcar/service/RacingCarService.kt
Original file line number Diff line number Diff line change
@@ -1,23 +1,30 @@
package step3.racingcar.service

import step3.racingcar.domain.Car
import step3.racingcar.domain.Cars
import step3.racingcar.domain.PlayInfo
import step3.racingcar.domain.Winners
import step3.racingcar.domain.RandomNumber
import step3.racingcar.view.ResultView.Companion.printRoundResult
import step3.racingcar.view.ResultView.Companion.printWinner

class RacingCarService {
class RacingCarService(private val randomNumber: RandomNumber) {
fun play(playInfo: PlayInfo) {
repeat(playInfo.totalRound) {
playEachRound(it, playInfo.cars)
}
printWinner(Winners.of(playInfo.cars))
printWinner(playInfo.cars)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

도메인 로직을 수행하는 Service는 View의 일을 가지고 있네요!
해당 로직은 controller로 옮기는 것은 어떨까요?

}

private fun playEachRound(currentRoundIndex: Int, cars: Cars) {
fun playEachRound(currentRoundIndex: Int, cars: Cars) {
cars.elements().forEach {
it.race(currentRoundIndex)
val randomNumber = randomNumber.value()
it.race(randomNumber)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cars에서 직접 값을 꺼내서 메서드를 호출하기 보다, cars에게 메시지를 던져보는 것은 어떨까요?

}
printRoundResult(currentRoundIndex, cars)
}

fun playEachRoundByCar(car: Car) {
val randomNumber = randomNumber.value()
car.race(randomNumber)
}
}
22 changes: 5 additions & 17 deletions src/main/kotlin/step3/racingcar/utils/RandomNumberGenerator.kt
Original file line number Diff line number Diff line change
@@ -1,23 +1,11 @@
package step3.racingcar.utils

import step3.racingcar.domain.Car
import step3.racingcar.domain.Cars
import step3.racingcar.domain.RandomNumber
import step3.racingcar.domain.RandomNumber.Companion.RANGE_END
import step3.racingcar.domain.RandomNumber.Companion.RANGE_START

object RandomNumberGenerator {
private const val RANGE_START = 1
private const val RANGE_END = 9

fun generateRandomNumberToCarByRound(cars: Cars, totalRound: Int) {
cars.elements().forEach {
generateRandomNumberToEachCar(it, totalRound)
}
}
class RandomNumberGenerator : RandomNumber {
override fun value(): Int = generate()

private fun generate(): Int = (RANGE_START..RANGE_END).random()

private fun generateRandomNumberToEachCar(car: Car, totalRound: Int) {
repeat(totalRound) {
car.addRandomNumber(generate())
}
}
}
5 changes: 2 additions & 3 deletions src/main/kotlin/step3/racingcar/view/ResultView.kt
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package step3.racingcar.view

import step3.racingcar.domain.Car
import step3.racingcar.domain.Cars
import step3.racingcar.domain.Winners

class ResultView {
companion object {
Expand Down Expand Up @@ -32,9 +31,9 @@ class ResultView {
return result
}

fun printWinner(winners: Winners) {
fun printWinner(cars: Cars) {
println()
println(WINNER_GUIDE_MESSAGE_FORMAT.format(winners.names))
println(WINNER_GUIDE_MESSAGE_FORMAT.format(cars.winnerNames()))
}
}
}
20 changes: 7 additions & 13 deletions src/test/kotlin/step3/racingcar/domain/CarTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -2,27 +2,24 @@ package step3.racingcar.domain

import io.kotest.core.spec.style.BehaviorSpec
import io.kotest.matchers.shouldBe
import step3.racingcar.fixture.CarFixtureGenerator

class CarTest : BehaviorSpec({
given("경주에 참가하는 자동차 한대에 4이상의 숫자가 주어지고") {
val currentRoundIndex = 0
val givenCar = CarFixtureGenerator.난수를_가지는_차량_생성("참가 차량", 4)
val givenCar = Car("참가 차량")

`when`("경주를 진행하면") {
givenCar.race(currentRoundIndex)
givenCar.race(4)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

given과 when이 조금 섞여있어 보여요.
given에는 필요한 차량만을 만들고 있고, when에서 4이상의 숫자를 넣고 있는데, when에 대한 내용이 given에 적혀있네요!

then("현재 차량의 주행거리를 1만큼 누적한다.") {
givenCar.distance shouldBe 1
}
}
}

given("경주에 참가하는 자동차 한대에 3이하의 숫자가 주어지고") {
val currentRoundIndex = 0
val givenCar = CarFixtureGenerator.난수를_가지는_차량_생성("참가 차량", 3)
val givenCar = Car("참가 차량")

`when`("경주를 진행하면") {
givenCar.race(currentRoundIndex)
givenCar.race(3)
then("현재 차량의 주행거리는 누적되지 않는다.") {
givenCar.distance shouldBe 0
}
Expand All @@ -33,22 +30,19 @@ class CarTest : BehaviorSpec({
val car = Car("참가 차량")

`when`("첫번째 라운드에 4가 주어지면") {
car.addRandomNumber(4)
car.race(0)
car.race(4)
then("차량 전진 횟수가 1 증가한다.") {
car.distance shouldBe 1
}
}
`when`("두번째 라운드에 3이 주어지면") {
car.addRandomNumber(3)
car.race(1)
car.race(3)
then("차량의 전진 횟수는 증가하지 않는다.") {
car.distance shouldBe 1
}
}
`when`("세번째 라운드에 6이 주어지면") {
car.addRandomNumber(6)
car.race(2)
car.race(6)
then("차량 전진 횟수가 1 증가한다.") {
car.distance shouldBe 2
}
Expand Down
28 changes: 28 additions & 0 deletions src/test/kotlin/step3/racingcar/domain/CarsTest.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package step3.racingcar.domain

import io.kotest.core.spec.style.BehaviorSpec
import io.kotest.matchers.shouldBe

internal class CarsTest : BehaviorSpec({
given("경주를 완료한 자동차 객체 목록이 주어지고") {
val 첫_번째_차량 = 경주를_완료한_차량_생성("첫 번째 차량", 1, 1, 1, 4, 4)
val 두_번째_차량 = 경주를_완료한_차량_생성("두 번째 차량", 1, 1, 4, 4, 4)
val 세_번째_차량 = 경주를_완료한_차량_생성("세 번째 차량", 4, 4, 4, 1, 1)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Car의 생성자에 distance 초기값을 넣게 만들어보는 것은 어떨까요?
그렇다면, 일일이 race를 호출하지 않고도, 원하는 위치에 존재하는 Car 객체를 만들어내서 손쉽게 테스트해볼 수 있겠네요 :)


`when`("우승자 객체를 생성하면") {
val 참가_차량_목록 = listOf(첫_번째_차량, 두_번째_차량, 세_번째_차량)
val given = Cars.of(참가_차량_목록)
then("우승한 차량들의 이름을 반환한다.") {
given.winnerNames() shouldBe "두 번째 차량, 세 번째 차량"
}
}
}
})

private fun 경주를_완료한_차량_생성(carName: String, vararg randomNumbers: Int): Car {
val car = Car(carName)
randomNumbers.forEach {
car.race(it)
}
return car
}
21 changes: 0 additions & 21 deletions src/test/kotlin/step3/racingcar/domain/WinnersTest.kt

This file was deleted.

26 changes: 0 additions & 26 deletions src/test/kotlin/step3/racingcar/fixture/CarFixtureGenerator.kt

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package step3.racingcar.fixture

import step3.racingcar.domain.RandomNumber

class TestRandomNumberGenerator(private val testValue: Int = DEFAULT_TEST_VALUE) : RandomNumber {
override fun value(): Int = testValue

companion object {
private const val DEFAULT_TEST_VALUE = 1
}
}
Loading