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] 자동차 경주(리팩토링) #1556

Open
wants to merge 2 commits into
base: mkkim90
Choose a base branch
from
Open
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
5 changes: 2 additions & 3 deletions src/main/kotlin/racingcar/CarApplication.kt
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import racingcar.controller.RacingController

private val racingController: RacingController = RacingController()
fun main() {
val joinRacingCar = racingController.join()
val playRacingCar = racingController.play(joinRacingCar)
racingController.winner(playRacingCar)
val playRacingCar = racingController.play()
racingController.win(playRacingCar)
}
29 changes: 15 additions & 14 deletions src/main/kotlin/racingcar/controller/RacingController.kt
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,24 @@ package racingcar.controller

import racingcar.domain.Cars
import racingcar.domain.WinnerCar
import racingcar.inputCarName
import racingcar.inputTryCount
import racingcar.view.printRacing
import racingcar.view.printWinner
import racingcar.service.RacingService
import racingcar.view.OutputView.printWinners
import racingcar.view.inputCarName

class RacingController {
fun join(): Cars = Cars.of(inputCarName().split(","))
class RacingController(
private val racingService: RacingService = RacingService()
) {
fun play(): Cars {
return racingService.play(join())
}

private fun join(): Cars = Cars.of(inputCarName().split(RACING_CAR_DELIMITER))
Copy link

Choose a reason for hiding this comment

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

자동차 이름을 입력받은 것을 컨트롤러 계층에서 분리하고 있네요. 즉 비즈니스 로직이 컨트롤러에서 수행되는 것인데, 이 경우 자동차 이름을 정상적으로 분리를 하는지에 대한 테스트를 하기 힘들 것 같아요. 🤔

Copy link
Author

Choose a reason for hiding this comment

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

Cars 클래스에 위임하겠습니다.


fun play(racingCars: Cars): Cars {
repeat(inputTryCount()) {
racingCars.move()
printRacing(racingCars)
}
return racingCars
fun win(playRacingCar: Cars) {
printWinners(WinnerCar.from(playRacingCar))
}

fun winner(playRacingCar: Cars) {
printWinner(WinnerCar.from(playRacingCar))
companion object {
private const val RACING_CAR_DELIMITER = ","
}
}
15 changes: 15 additions & 0 deletions src/main/kotlin/racingcar/service/RacingService.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package racingcar.service

import racingcar.domain.Cars
import racingcar.view.OutputView
import racingcar.view.inputTryCount

class RacingService {
fun play(racingCars: Cars): Cars {
repeat(inputTryCount()) {
racingCars.move()
OutputView.printRacing(racingCars)
Copy link

Choose a reason for hiding this comment

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

지금 구조를 보면 의존성의 흐름이 Controller -> Service -> View Layer입니다.
보통 의존성은 밖에서 안으로 단방향으로 흐르는 편이에요. 즉 서비스 계층에서 출력 계층으로 의존성이 흐르는건 청신호는 아니죠. 서비스 계층이라면 테스트 객체인데, 저희는 콘솔창에 어떻게 출력되는지를 테스트할건 아니니까요(요구사항중 UI는 테스트하지 않는다는 내용도 있었죠. )

이런 출력 계층에 대한 정보는 컨트롤러 이상이 알 필요가 없도록 고쳐보면 좋을 것 같아요

Copy link
Author

Choose a reason for hiding this comment

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

controller 와 service 간에 레이싱 스냅샷 데이터를 출력할 수 있는 DTO 를 추가했습니다

}
return racingCars
}
}
2 changes: 1 addition & 1 deletion src/main/kotlin/racingcar/view/InputView.kt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package racingcar
package racingcar.view

fun inputTryCount(): Int {
println("시도할 횟수는 몇 회인가요?")
Expand Down
26 changes: 17 additions & 9 deletions src/main/kotlin/racingcar/view/OutputView.kt
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,24 @@ package racingcar.view
import racingcar.domain.Car
import racingcar.domain.Cars

fun printRacing(racingCars: Cars) {
racingCars.cars.forEach {
println("${it.name} : ${"-".repeat(maxOf(0, it.position))}")
object OutputView {

private const val WINNER_JOIN_SEPARATOR = ","
private const val RACING_COURSE = "-"
fun printRacing(racingCars: Cars) {
val result = buildString {
racingCars.cars.forEach {
Copy link

Choose a reason for hiding this comment

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

a.b.c() 현재 a라는 객체에서 c라는 함수호출에 대해 알 필요가 있을까요? 디미터 법칙에 위배됩니다.
또한 OOP관점으로 볼 때 우리는 객체간에 요청과 응답을 하면서 서로의 내부정보를 알 필요 없도록하도록 캡슐화를 제공하는데, 이렇게 내부 값을 꺼내어 연산을 수행하면 캡슐화가 깨지게 되겠죠!

racingCar.contentForEach { ... } 이런식으로 함수를 만들어서 제공해도 좋을 것 같아요.

Copy link
Author

Choose a reason for hiding this comment

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

DTO에게 출력 문자열을 위임했습니다.

appendLine("${it.name} : ${RACING_COURSE.repeat(maxOf(0, it.position))}")
}
appendLine()
}
println(result)
}
println()
}

fun printWinner(winnerCars: List<Car>): String = buildString {
val winnersCarNames = winnerCars
.joinToString(",") { it.name }
fun printWinners(winnerCars: List<Car>): String = buildString {
val winnersCarNames = winnerCars
.joinToString(WINNER_JOIN_SEPARATOR) { it.name }

println("${winnersCarNames}가 최종 우승했습니다.")
println("${winnersCarNames}가 최종 우승했습니다.")
}
}