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

[Step4] - 자동차 경주(우승자) #909

Open
wants to merge 10 commits into
base: riyenas0925
Choose a base branch
from
11 changes: 7 additions & 4 deletions src/main/kotlin/racingcar/Application.kt
Original file line number Diff line number Diff line change
@@ -1,16 +1,19 @@
package racingcar

class Application

fun main() {
val (cars, roundCount) = InputView()
val cars = InputView.readCars()
val roundCount = InputView.readRoundCount()

val racingGame = RacingGame(
cars = cars,
numberGenerator = RacingCarNumberGenerator()
)

repeat(roundCount) {
val drivingCars = racingGame.round()
ResultView.print(drivingCars)
ResultView.printLocation(drivingCars)
}

val winners = racingGame.judge()
ResultView.printWinner(winners)
}
25 changes: 18 additions & 7 deletions src/main/kotlin/racingcar/Car.kt
Original file line number Diff line number Diff line change
@@ -1,16 +1,27 @@
package racingcar

class Car(
private var location: Int = 0,
private var _location: Int = DEFAULT_CAR_LOCATION,
val name: String,
) {
fun move(randNumber: Int) {
val rule = MovingJudgeRule.judge(randNumber)
val strategy = rule.strategy()
init {
require(isValidLength(name)) { "글자수는 5자를 넘을 수 없어요" }
Copy link

Choose a reason for hiding this comment

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

객체가 생성되는 시점에 유효성을 체크하도록 만드셨군요? 👍👍
개인적으로 지금의 경우에는 Validation 로직이 간단하므로 메서드로 추출할 필요없이 바로 명시하는 것이 더 가독성이 좋은 것 같아요. 🙂

추가적으로 예외 메시지를 조금 더 명확하게 적어보면 어떨까요? 😃

Suggested change
require(isValidLength(name)) { "글자수는 5자를 넘을 수 없어요" }
require(name.length <= VALID_CAR_NAME_LENGTH) { "자동차 이름은 5자를 넘을 수 없어요" }

}

val location
get() = _location
Comment on lines +7 to +12
Copy link

Choose a reason for hiding this comment

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

Kotlin Coding Convention에 따르면 클래스는 아래의 순서대로 포맷을 지키라고 하고있어요! 😃

  1. Property declarations and initializer blocks
  2. Secondary constructors
  3. Method declarations
  4. Companion object
Suggested change
init {
require(isValidLength(name)) { "글자수는 5자를 넘을 수 없어요" }
}
val location
get() = _location
val location
get() = _location
init {
require(isValidLength(name)) { "글자수는 5자를 넘을 수 없어요" }
}

fun move(number: Int) {
val rule = MovingJudgeRule.judge(number)
Copy link

Choose a reason for hiding this comment

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

이 부분에 대해서도 다시 고민해봤는데 (꼭 강한 결합 때문만이 아니라) MovingJudgeRule 객체가 Car 객체의 상태를 변경하는 것이 맞을까? 🤔 라고 생각했을 때, 객체지향의 세계에서 객체는 자율적인 존재이고 객체는 스스로의 행동에 의해서만 상태가 변경되어야하고 또한 자동차가 움직이는 행위는 온전한 자동차의 역할과 책임이기 때문에 자동차 객체가 스스로 결정하는 것이 더 좋다고 생각하는데, 동민님은 어떻게 생각하시나요? 🤔

참고자료 - 상태 부분

val strategy = rule.strategy
_location = strategy.move(_location)
}
Comment on lines +4 to +17
Copy link
Author

@riyenas0925 riyenas0925 Nov 20, 2022

Choose a reason for hiding this comment

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

backing properties를 사용해서 구현했는데 클래스 내부에서 location과 _location이 헷갈리기도 하고 무엇보다 변수 앞에 _ 가 붙는게 익숙치 않다는 느낌을 받았어요. 리뷰어님은 어떤 방식을 선호하시는지 궁금해요~!

Copy link

Choose a reason for hiding this comment

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

저 같은 경우에는 var location 프로퍼티와 같이 상태 값이 기본형이라면 Step2 코멘트 내용 중에 두 번째 방식인 setter만 막는 방법을 사용하는 편입니다. 🙂

저는 backing properties를 잘 사용하지 않는 편이에요. 그 이유는 대부분은 val로 프로퍼티를 선언하여 변경되지 않는 값으로 만드는 편이기 때문인데, 그럼에도 불구하고 _를 쓰는 경우는 MutableList와 같이 가변 Collection 타입이 객체의 프로퍼티로 있는 경우, 이 값은 외부에서 변경이 될 수도 있기 때문에 이를 방지하고자 _로 실제 프로퍼티를 가지고 외부에는 _를 제외한 값으로 ReadOnly용 값을 만들어서 방어적인 코드를 작성하려고하는 편이에요. 😃


location = strategy.move(location)
private fun isValidLength(name: String): Boolean {
return name.length <= VALID_CAR_NAME_LENGTH
}

fun currentLocation(): Int {
return location
companion object {
private const val DEFAULT_CAR_LOCATION = 0
private const val VALID_CAR_NAME_LENGTH = 5
}
}
36 changes: 19 additions & 17 deletions src/main/kotlin/racingcar/InputView.kt
Original file line number Diff line number Diff line change
@@ -1,24 +1,26 @@
package racingcar

data class InputView(
val cars: List<Car> = readCars("자동차 대수는 몇 대인가요?"),
val roundCount: Int = readRoundCount("시도할 횟수는 몇 회인가요?"),
) {
companion object {
private fun readCars(message: String): List<Car> {
println(message)
object InputView {
fun readCars(): List<Car> {
Copy link

Choose a reason for hiding this comment

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

read라는 표현보다는 input이라는 표현이 더 맞지 않을까요? 🤔

println("경주할 자동차 이름을 입력하세요(이름은 쉼표(,)를 기준으로 구분).")
val names = parseCarName(readln())

val list = mutableListOf<Car>()
repeat(readln().toInt()) {
list.add(Car())
}

return list
val list = mutableListOf<Car>()
for (name in names) {
list.add(
Car(name = name)
)
}
Comment on lines +8 to 13
Copy link

Choose a reason for hiding this comment

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

아래와 같이 변경해볼 수 있을 것 같아요. 😃

return return names.map { Car(name = it) }


private fun readRoundCount(message: String): Int {
println(message)
return readln().toInt()
}
return list
}

fun readRoundCount(): Int {
println("시도할 횟수는 몇 회인가요?")
return readln().toInt()
}

private fun parseCarName(input: String): List<String> {
return input.split(",")
}
}
10 changes: 3 additions & 7 deletions src/main/kotlin/racingcar/MovingJudgeRule.kt
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@ package racingcar

enum class MovingJudgeRule(
private val expression: (Int) -> (Boolean),
private val strategy: MovingStrategy
val strategy: MovingStrategy
Copy link

Choose a reason for hiding this comment

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

3단계 미션에서 코멘트 드렸었는데 반영이 안된 것 같아요. 🥲

먼저 MovingJudgeRule 객체의 상태 값들의 네이밍이 어떤 역할을 하는지 감이 안잡히는 것 같아요. 🤔
역할을 한눈에 파악할 수 있는 네이밍을 고려해서 변경해보면 좋을 것 같아요. 😉

그리고 MovingJudgeRuleMovingStrategy는 강한 결합을 가지고 있어서 새로운 MovingJudgeRule이 만들어지면 자연스럽게 새로운 MovingStrategy도 생성되게 될거 같아요. 🤔 이 두 객체는 서로 분리하는 것 보다는 MovingStrategy가 가지는 프로퍼티를 그대로 MovingJudgeRule이 (MovingStrategy 객체 대신에) 가지도록 두는 것이 좋을 것 같아요.

Suggested change
val strategy: MovingStrategy
val strategy: (Int) -> (Int)

) {

ADVANCE_RULE(
expression = { number -> number >= FORWARD_STRATEGY_BOUND },
expression = { it >= FORWARD_STRATEGY_BOUND },
strategy = MovingStrategy.ADVANCE,
),
STOP_RULE(
expression = { false },
expression = { it < FORWARD_STRATEGY_BOUND },
strategy = MovingStrategy.STOP,
),
;
Expand All @@ -19,10 +19,6 @@ enum class MovingJudgeRule(
return expression(number)
}

fun strategy(): MovingStrategy {
return strategy
}

companion object {
private const val FORWARD_STRATEGY_BOUND = 4

Expand Down
7 changes: 6 additions & 1 deletion src/main/kotlin/racingcar/NumberGenerator.kt
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ interface NumberGenerator {

class RacingCarNumberGenerator : NumberGenerator {
Copy link

Choose a reason for hiding this comment

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

NumberGenerator 객체는 상태를 가지는 것도 아니기 때문에 굳이 객체로 만들 필요가 없을 것 같아요. 🤔
Util성 함수로 만들어도 충분하지 않을까요? 🙂

override fun rand(): Int {
return (0..9).random()
return (RAND_NUMBER_START_BOUND..RAND_NUMBER_END_BOUND).random()
}

companion object {
private const val RAND_NUMBER_START_BOUND = 0
private const val RAND_NUMBER_END_BOUND = 9
}
}
6 changes: 5 additions & 1 deletion src/main/kotlin/racingcar/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,15 @@
- [x] 숫자 생성기 0 ~ 9 사이의 무작위한 값을 생성해요.

### 레이싱 게임 (Racing Game)
- [x] 레이싱 게임에서 참여하는 자동차는 1대 이상이어야 합니다.
- [x] 레이싱 게임은 자동차의 대수와 시도할 횟수를 입력할 수 있어요.
- [x] 레이싱 게임은 매 횟수마다 자동차의 현재 위치를 보여줘요.
- [x] 레이싱 게임은 자동차의 이름을 쉽표로 구분해요.
- [x] 레이싱 게임은 매 횟수마다 자동차의 이름과 현재 위치를 보여줘요.

#### 자동차 (Car)
- [x] 자동차는 입력한 숫자에 따라 이동해요.
- [x] 0~3의 숫자를 입력하면 자동차는 멈춰요.
- [x] 4 이상의 숫자를 입력하면 자동차는 1만큼 전진해요.
- [x] 자동차는 이름을 가질 수 있어요.
- [x] 자동차의 이름은 5자를 초과할 수 없어요.
- [x] 자동차는 자신이 이동한 거리를 알 수 있어요.
16 changes: 16 additions & 0 deletions src/main/kotlin/racingcar/RacingGame.kt
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@ class RacingGame(
private val cars: List<Car>,
private val numberGenerator: NumberGenerator,
) {

init {
requireCarExist(cars)
}

fun round(): List<Car> {
for (car in cars) {
val randNumber = numberGenerator.rand()
Expand All @@ -12,4 +17,15 @@ class RacingGame(

return cars
}

fun judge(): List<Car> {
Copy link

Choose a reason for hiding this comment

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

judge라는 메서드 네이밍이 자주 보이는 것 같아요. 🤔
조금 더 의미있는 메서드 네이밍을 지어보면 어떨까요? 😃

좋은 코드를 위한 자바 메서드 네이밍

val maxLocation = cars.maxOf { it.location }
return cars.filter {
it.location == maxLocation
}
}

private fun requireCarExist(cars: List<Car>) {
require(cars.isNotEmpty()) { "레이싱 게임에서 참여하는 자동차는 1대 이상이어야 합니다." }
}
}
19 changes: 11 additions & 8 deletions src/main/kotlin/racingcar/ResultView.kt
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
package racingcar

class ResultView {
companion object {
fun print(cars: List<Car>) {
for (car in cars) {
val location = car.currentLocation()
repeat(location) { print("-") }
println()
}
object ResultView {
fun printLocation(cars: List<Car>) {
for (car in cars) {
val location = car.location
print(car.name + " : ")
repeat(location) { print("-") }
println()
Comment on lines +7 to 9
Copy link

Choose a reason for hiding this comment

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

아래와 같이 바꿔볼 수 있을 것 같아요. 😃

Suggested change
print(car.name + " : ")
repeat(location) { print("-") }
println()
println("${car.name} : ${"-".repeat(car.location)}")

}
println()
}

fun printWinner(cars: List<Car>) {
println("${cars.joinToString { it.name }}가 최종 우승했습니다.")
}
}
12 changes: 6 additions & 6 deletions src/test/kotlin/racingcar/CarTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -6,25 +6,25 @@ import io.kotest.matchers.shouldBe
class CarTest : StringSpec({

"자동차의 처음 위치는 0이에요" {
val car = Car()
car.currentLocation() shouldBe 0
val car = Car(name = "car")
car.location shouldBe 0
}

"자동차는 4 ~ 9의 숫자일 때 1 만큼 이동해요" {
(4..9).forEach { number ->
val car = Car()
val car = Car(name = "car")
car.move(number)

car.currentLocation() shouldBe 1
car.location shouldBe 1
}
}

"자동차는 0~3의 숫자일 때 이동하지 않아요" {
(0..3).forEach { number ->
val car = Car()
val car = Car(name = "car")
car.move(number)

car.currentLocation() shouldBe 0
car.location shouldBe 0
}
}
})
18 changes: 16 additions & 2 deletions src/test/kotlin/racingcar/RacingGameTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,33 @@ import io.kotest.core.spec.style.StringSpec
import io.kotest.matchers.shouldBe
import io.mockk.every
import io.mockk.mockk
import org.junit.jupiter.api.assertThrows
import java.lang.IllegalArgumentException

class RacingGameTest : StringSpec({
"레이싱 게임은 매 횟수마다 자동차의 현재 위치를 보여줘요" {
val mockNumberGenerator = mockk<NumberGenerator>()
Copy link

Choose a reason for hiding this comment

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

이번 미션에서는 mock 라이브러리 없이 테스트 코드를 작성해보시면 좋을 것 같아요. 😃
gradle에서 mockk 라이브러리 의존성을 제거해주세요. 🙏

every { mockNumberGenerator.rand() } returns 0

val racingGame = RacingGame(
cars = listOf(Car(0)),
cars = listOf(Car(0, "car")),
numberGenerator = mockNumberGenerator
)

val car = racingGame.round()[0]

car.currentLocation() shouldBe 0
car.location shouldBe 0
}

"레이싱 게임에 참여하는 자동차는 1대 이상이어야 합니다." {
assertThrows<IllegalArgumentException> {
val mockNumberGenerator = mockk<NumberGenerator>()
every { mockNumberGenerator.rand() } returns 0

val racingGame = RacingGame(
cars = listOf(),
numberGenerator = mockNumberGenerator
)
}
}
})