-
Notifications
You must be signed in to change notification settings - Fork 120
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단계 자동차 경주 제출합니다. #64
Changes from 80 commits
4d15ac7
2fb4547
d85e64e
69fe601
ca51779
37462f6
8808d37
1d86cdd
e1f5fc1
118d2cb
b826b0e
972d412
7b51f8c
b16841c
e724948
1381ae5
d88ea80
7e42178
0edf628
1ca0cda
4d5d28b
89bfc95
fe11d89
5d67e4e
3571323
39f2cdd
3e46229
c34e4cc
890bef6
8945701
3aa878a
74f7af6
3ab052f
ce2e1bb
efd5d0c
9282f50
35dff55
e45ab6b
4f9ba23
07dcf8e
983fd98
e735d17
8a24159
d9c1cde
3250ce8
c41adbe
aaca844
385e9b7
7012826
810050b
0924fe7
1f76b4e
3783a31
f3d42d2
a2441a2
3966720
f1d7a52
5a27b6d
cbed4f1
919ba1c
7e22dea
322b011
b96c5d1
3cd5d3b
b354f94
1b9aae3
a2a3628
c1bbd0c
0e2a0d6
330601f
910225c
70e8c98
5f2d1f7
feb31e3
2a702a1
9298caf
16de727
6320815
14595f1
249b12d
9656256
ee98952
4cf09d2
ba98fd4
a614450
255a8bf
228132e
4b82d73
782af39
6360209
9c847c0
8046843
a39cc01
3a09e61
78fe97b
24e8396
b2919dd
dc441fa
8268211
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,27 @@ | ||||||||||||
package racingcar.model.car | ||||||||||||
|
||||||||||||
open class Car(name: String, _position: Int = 0) { | ||||||||||||
var carName: CarName = CarName(name) | ||||||||||||
private set | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
그냥 불변으로 선언 하셔도 좋을거 같아요! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 헉 이런 실수를..! |
||||||||||||
var position: Int = _position | ||||||||||||
private set | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||
|
||||||||||||
fun moveRandomly(moveCondition: Int) { | ||||||||||||
if (moveCondition >= MOVEMENT_PROBABILITY) { | ||||||||||||
++position | ||||||||||||
} | ||||||||||||
} | ||||||||||||
|
||||||||||||
fun toDto(): CarDto = CarDto(carName.name) | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Car객체에 CarDto에 대한 의존도가 생겼네요, |
||||||||||||
|
||||||||||||
companion object { | ||||||||||||
private const val MOVEMENT_PROBABILITY = 4 | ||||||||||||
} | ||||||||||||
} | ||||||||||||
|
||||||||||||
class CarDto(_carName: String, val position: Int = 0) { | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Dto 클래스들을 분리해주세요 😄 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 네! Dto와 Domain model을 분리하고 mapper 클래스 파일을 만들어 변환을 돕는 코드로 바꿔 보겠습니다 🙂 |
||||||||||||
var carName: CarName = CarName(_carName.trim()) | ||||||||||||
private set | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 변경이 되지않는 변수에는 기본적으로 항상 val을 사용해주세요! |
||||||||||||
|
||||||||||||
fun toModel(): Car = Car(carName.name, position) | ||||||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,25 +1,18 @@ | ||
package racingcar.model | ||
package racingcar.model.car | ||
|
||
class Car(val name: String) { | ||
var position: Int = 0 | ||
private set | ||
class CarName(_name: String) { | ||
val name: String | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. carName.name이라는 중복적인 name이 등장하게되는데 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 오..! name이 중복되는 네이밍보다 value를 사용하는 것에 훨씬 보기 좋아요! |
||
|
||
init { | ||
name = _name.trim() | ||
require(name.length in MIN_CAR_NAME_LENGTH..MAX_CAR_NAME_LENGTH) { | ||
throw IllegalArgumentException(CAR_NAME_LENGTH_OVER_BOUNDARY_ERROR_MESSAGE) | ||
} | ||
} | ||
|
||
fun move(condition: Int) { | ||
if (condition >= MOVEMENT_PROBABILITY) { | ||
++position | ||
CAR_NAME_LENGTH_OVER_BOUNDARY_ERROR_MESSAGE | ||
} | ||
} | ||
|
||
companion object { | ||
private const val MIN_CAR_NAME_LENGTH = 1 | ||
private const val MAX_CAR_NAME_LENGTH = 5 | ||
private const val MOVEMENT_PROBABILITY = 4 | ||
|
||
private const val CAR_NAME_LENGTH_OVER_BOUNDARY_ERROR_MESSAGE = | ||
"자동차 이름 길이의 범위는 $MIN_CAR_NAME_LENGTH 이상 $MAX_CAR_NAME_LENGTH 이하입니다." | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
package racingcar.model.car | ||
|
||
import racingcar.utils.random.RandomGenerator | ||
|
||
class Cars(_cars: List<Car>) : List<Car> by _cars { | ||
init { | ||
validateExistDuplicatedCarName() | ||
} | ||
|
||
private fun validateExistDuplicatedCarName() { | ||
val nonDuplicatedCarsForName = this.distinctBy { it.carName.name } | ||
|
||
require(this.size == nonDuplicatedCarsForName.size) { | ||
DUPLICATED_CAR_NAME_ERROR_MESSAGE | ||
} | ||
} | ||
|
||
fun moveAllRandomly(movementProbabilityGenerator: RandomGenerator): Cars = this.onEach { car -> | ||
val moveProbability = movementProbabilityGenerator.generate() | ||
car.moveRandomly(moveProbability) | ||
} | ||
|
||
fun getWinners(): Winners { | ||
val winnerStandard = this.maxBy { it.position } | ||
return Winners(this.filter { it.position == winnerStandard.position }) | ||
} | ||
|
||
fun toDto(): CarsDto = CarsDto( | ||
this.map { car -> car.toDto() } | ||
) | ||
|
||
companion object { | ||
private const val DUPLICATED_CAR_NAME_ERROR_MESSAGE = | ||
"중복된 자동차 이름이 존재합니다." | ||
} | ||
} | ||
|
||
class CarsDto(_cars: List<CarDto>) : List<CarDto> by _cars { | ||
fun toModel(): Cars = Cars(this.map { it.toModel() }) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
package racingcar.model.car | ||
|
||
class Winners(winners: List<Car>) : List<Car> by winners { | ||
fun toDto(): WinnersDto = WinnersDto(this) | ||
} | ||
|
||
class WinnersDto(winners: List<Car>) : List<Car> by winners |
This file was deleted.
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,38 +1,25 @@ | ||
package racingcar.service | ||
|
||
import racingcar.model.Car | ||
import racingcar.repository.CarRepository | ||
import racingcar.repository.Repository | ||
import racingcar.model.car.CarsDto | ||
import racingcar.model.car.WinnersDto | ||
import racingcar.model.round.RoundDto | ||
import racingcar.utils.random.MovementProbabilityGenerator | ||
import racingcar.utils.random.RandomGenerator | ||
|
||
class RacingService( | ||
private val carRepository: Repository<Car> = CarRepository() | ||
_cars: CarsDto, | ||
private val movementProbabilityGenerator: RandomGenerator = MovementProbabilityGenerator() | ||
) { | ||
fun getAll(): List<Car> = carRepository.selectAll() | ||
private val cars = _cars.toModel() | ||
|
||
fun insertCars(cars: List<Car>) { | ||
cars.forEach { insertCar(it) } | ||
fun runAllRounds(round: RoundDto, doEachRoundResult: (CarsDto) -> Unit) { | ||
repeat(round.toModel().count) { | ||
doEachRoundResult(moveCarsRandomly()) | ||
} | ||
} | ||
|
||
private fun insertCar(car: Car) = carRepository.insert(car) | ||
private fun moveCarsRandomly(): CarsDto = | ||
cars.moveAllRandomly(movementProbabilityGenerator).toDto() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (이 부분은 정답이 없는 부분이니 참고하셔서 고민만 해주세요 ) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이 글을 읽어보고 '어떻게 유지보수에 유연한 코드를 작성할 수 있을까?' 라는 고민을 할 수 있었습니다.
말씀해주신 것처럼 주어진 요구사항이 변경되었을 때, 기존 코드를 변경해야 하는 부분이 정말 많이 생긴다는 사실을 알 수 있었습니다.
이러한 구현 방식으로 리팩토링 하다보니 자동차 이동 조건이 변경되었을 때, CarMoveCondtion을 구현하는 또 다른 조건 클래스를 주입해주기만 하면 되는 구조로 바뀌었습니다!
sealed 클래스를 사용하여 몇 칸 움직일지 정하는 부분을 OCP에 위배되지 않게끔 시도해봤습니다. 긴 글 읽어주셔서 감사합니다! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 👍 |
||
|
||
fun createCars(names: List<String>): List<Car> = | ||
names.map { Car(it) } | ||
|
||
fun moveRandomly(car: Car) { | ||
car.move(getRandomProbability()) | ||
} | ||
|
||
private fun getRandomProbability(): Int = | ||
(START_RANDOM_MOVEMENT_PROBABILITY..END_RANDOM_MOVEMENT_PROBABILITY).random() | ||
|
||
fun getWinners(): List<Car> { | ||
val cars = getAll() | ||
val winnerStandard = cars.maxBy { it.position } | ||
return cars.filter { it.position == winnerStandard.position } | ||
} | ||
|
||
companion object { | ||
private const val START_RANDOM_MOVEMENT_PROBABILITY = 1 | ||
private const val END_RANDOM_MOVEMENT_PROBABILITY = 10 | ||
} | ||
fun getWinners(): WinnersDto = cars.getWinners().toDto() | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
package racingcar.utils | ||
|
||
fun List<String>.removeBlank() = map { it.trim() } | ||
fun List<String>.removeBlank(): List<String> = | ||
this.map { it.trim() } |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
package racingcar.utils.random | ||
|
||
class MovementProbabilityGenerator : RandomGenerator { | ||
override fun generate(): Int = | ||
(START_RANDOM_MOVEMENT_PROBABILITY..END_RANDOM_MOVEMENT_PROBABILITY).random() | ||
|
||
companion object { | ||
private const val START_RANDOM_MOVEMENT_PROBABILITY = 1 | ||
private const val END_RANDOM_MOVEMENT_PROBABILITY = 10 | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
package racingcar.utils.random | ||
|
||
interface RandomGenerator { | ||
fun generate(): Int | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 인터페이스에서는 Random으로 생성된다는 정보를 포함하는게 좋은 방법일까요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 아.. 그렇군요! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mock 라이브러리는 저도 자주 사용하는 라이브러리인데요,
이번 미션에서는 mock라이브러리 사용하지 않고 테스트 코드를 짜보는걸 목표로 해보는게 어떨까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
네! 한 번 해보겠습니다 :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
추천해주신 것처럼 mock 라이브러리를 제거하고 Fake 객체를 사용하는 방식으로 구현을 시도해보았습니다!
이동 조건이 변경, 추가되었을 때 확장하기 편한 방식으로 구현하다보니, 기존에 RandomGenerator를 제거하고, CarRandomMoveCondition.class를 새롭게 구현하였습니다.
구글링을 통해 찾아보니 Fake 객체를 해당 클래스 또는 인터페이스 내에 구현하는 분들이 꽤 많았습니다.
여기서 궁금했던 점은...
입니다..!
구현 방식이 정형화되어 있지 않다보니 어떤게 맞고 틀리다고 할 수 없지만 보편적으로 사용되는 방식이 있는지 궁금해졌습니다!
그리고 테스트를 위한 코드를 지양하라고 알고 있는데 Fake 객체는 그 범주에서 예외인지 의문이 들었습니다.
현직자의 입장에서 의견을 듣고 싶습니다..!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
실제 패키지 내에서 테스트를 위한 코드는 지양하는것이 좋습니다.
저는 Fake객체는 test 패키지 범위에서 관리되기 때문에 실제 구현이라고 보지는 않기도 합니다!
실제 프로젝트에는 객체 생성이 어렵거나, Fake가 힘든 케이스도 있기때문에, Mock 라이브러리를 많이 사용하긴 합니다.
하지만, 실제 간단한 경우에는 Fake객체 생성을 하는 경우도 많습니다 👍