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

[둘리] 2단계 자동차 경주 제출합니다. #68

Merged
merged 12 commits into from
Feb 15, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Prev Previous commit
Next Next commit
refactor(CarsInfo): CarsInfo 삭제, Cars/Car 클래스만 사용
  • Loading branch information
hyemdooly committed Feb 12, 2023
commit cb95bad275cd85e8af123016575f3b614c61ed84
2 changes: 1 addition & 1 deletion src/main/kotlin/controller/RaceGame.kt
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ class RaceGame(private val outputView: OutputView, private val inputView: InputV
private fun tryMove(cars: Cars) {
repeat(cars.getCarSize()) {
cars.move(it)
outputView.outputResult(cars.getCarInfo(it))
outputView.outputResult(cars.getCar(it))
}
outputView.outputNextLine()
}
Expand Down
13 changes: 8 additions & 5 deletions src/main/kotlin/model/Car.kt
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,19 @@ package model

import util.Validator

class Car(private val name: String, private var position: Int = 0) {
class Car(private val _name: String, private var _position: Int = 0) {

Choose a reason for hiding this comment

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

코틀린에서 변수에 언더바(_)를 붙이는 것은, backing property를 사용할 때 붙여요.
생성자에 언더바를 붙이지 않고 backing property를 사용하려면 아래와 같이 하시면 됩니다. (이하 관련 내용 동일)

class Car(val name: String, position: Int) {
    private var _position: Int = position
    val position: Int get() = _position
}

ref. https://kotlinlang.org/docs/coding-conventions.html#names-for-backing-properties


val name: String
get() = _name
val position: Int
get() = _position

init {
Validator().checkName(name)
Validator().checkName(_name)
}

fun getInfo(): CarInfo = CarInfo(name, position)

fun move(condition: Int) {
if (condition >= MOVING_POINT) position++
if (condition >= MOVING_POINT) _position++
}

companion object {
Expand Down
3 changes: 0 additions & 3 deletions src/main/kotlin/model/CarInfo.kt

This file was deleted.

19 changes: 7 additions & 12 deletions src/main/kotlin/model/Cars.kt
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,15 @@ package model

import generator.RandomGenerator

class Cars(private val cars: List<Car>) {
constructor(input: String) : this(input.split(",").mapIndexed { _, name -> Car(name.trim()) })
class Cars(private val _cars: List<Car>) {
val cars: List<Car>
get() = _cars

fun getCarInfos(): List<CarInfo> {
val carInfos = mutableListOf<CarInfo>()
repeat(cars.size) {
carInfos.add(cars[it].getInfo())
}
return carInfos
}
constructor(input: String) : this(input.split(",").mapIndexed { _, name -> Car(name.trim()) })

fun getCarInfo(index: Int): CarInfo = cars[index].getInfo()
fun getCarSize(): Int = cars.size
fun getCar(index: Int): Car = _cars[index]
fun getCarSize(): Int = _cars.size
fun move(index: Int) {
cars[index].move(RandomGenerator().getRandomNumber())
_cars[index].move(RandomGenerator().getRandomNumber())
}

Choose a reason for hiding this comment

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

move 함수를 잘 구현해주셨네요. 👍
하지만 랜덤값을 생성하는 로직이 함수 내부에 존재하게 되면서, 이 함수가 동작하는 것을 제대로 테스트하기가 어려워보여요.
함수가 의도하는 기능을 테스트할 수 있도록 코드를 개선해보면 어떨까요?

}
3 changes: 1 addition & 2 deletions src/main/kotlin/util/CarsHelper.kt
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@ import model.Cars

object CarsHelper {

Choose a reason for hiding this comment

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

객체 이름만 봤을 때는 "자동차들의 도우미"로 해석이 되는데요.
여기서 Helper라는 단어는 포괄적인 의미로 사용돼요.
클래스에 정의된 함수의 역할에 맞게 클래스명을 작성해보면 어떨까요?

fun findWinners(cars: Cars): List<String> {
val carInfos = cars.getCarInfos()
val equalCars = carInfos.groupBy({ it.position }, { it.name })
val equalCars = cars.cars.groupBy({ it.position }, { it.name })
return equalCars[equalCars.keys.max()]?.toList() ?: listOf()

Choose a reason for hiding this comment

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

방어적 복사 활용 👍
추가로 빈 리스트를 리턴하고 싶으시다면, listOf()보다는 emptyList()가 좀 더 가독성이 좋습니다.

}
}
8 changes: 4 additions & 4 deletions src/main/kotlin/view/OutputView.kt
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package view

import model.CarInfo
import model.Car
import util.Constants

class OutputView {
Expand All @@ -26,9 +26,9 @@ class OutputView {
println(error)
}

fun outputResult(info: CarInfo) {
print(info.name + " : ")
repeat(info.position) {
fun outputResult(car: Car) {
print(car.name + " : ")
repeat(car.position) {
print("-")
}
println()
Expand Down
10 changes: 5 additions & 5 deletions src/test/kotlin/CarTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@ import org.junit.jupiter.api.assertThrows
class CarTest {

@Test
fun getInfoTest() {
fun initTest() {

Choose a reason for hiding this comment

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

지금과 같은 초기화 테스트보다는, 내가 원하는 자동차 이름을 넣었을 때 해당 이름으로 객체가 생성되는지를 확인한다는 의미를 담아보면 어떨까요?

val car = Car("dool")
assertThat(car.getInfo().name).isEqualTo("dool")
assertThat(car.getInfo().position).isEqualTo(0)
assertThat(car.name).isEqualTo("dool")
assertThat(car.position).isEqualTo(0)
}

@Test
Expand All @@ -23,13 +23,13 @@ class CarTest {
fun moveTest() {
val car = Car("dool")
car.move(3)
assertThat(car.getInfo().position).isEqualTo(0)
assertThat(car.position).isEqualTo(0)
}

@Test
fun dontMoveTest() {
val car = Car("dool")
car.move(4)
assertThat(car.getInfo().position).isEqualTo(1)
assertThat(car.position).isEqualTo(1)
}
}
2 changes: 1 addition & 1 deletion src/test/kotlin/CarsTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ class CarsTest {
fun mappingCarsTest() {
val carNames = listOf("pobi", "dool", "woni")
repeat(cars.getCarSize()) {
assertThat(cars.getCarInfo(it).name).isEqualTo(carNames[it])
assertThat(cars.getCar(it).name).isEqualTo(carNames[it])
}
}

Expand Down