-
Notifications
You must be signed in to change notification settings - Fork 410
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] 자동차 경주(리팩터링) #1752
base: saerang
Are you sure you want to change the base?
[step5] 자동차 경주(리팩터링) #1752
Changes from 2 commits
27381df
a221c19
ac109d9
cb0a730
b7b7742
065749b
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 |
---|---|---|
@@ -1,33 +1,19 @@ | ||
package racingcar | ||
|
||
import racingcar.domain.Cars | ||
import racingcar.domain.Winners | ||
import racingcar.util.RandomNumberGenerator | ||
import racingcar.view.InputView | ||
import racingcar.view.OutputView | ||
|
||
fun main() { | ||
println("경주할 자동차 이름을 입력하세요(이름은 쉼표(,)를 기준으로 구분).") | ||
val usernames: String = readln() | ||
println("시도할 횟수는 몇 회인가요?") | ||
val round: Int = readln().toInt() | ||
val usernames = InputView.carNames() | ||
val round = InputView.roundCount() | ||
|
||
val cars = Cars.usernames(usernames) | ||
repeat(round) { | ||
cars.race(RandomNumberGenerator()) | ||
gameResult(cars) | ||
OutputView.gameResult(cars) | ||
} | ||
|
||
gameWinner(cars) | ||
} | ||
|
||
private fun gameResult(cars: Cars) { | ||
cars.cars().forEach { | ||
println(it.username() + " : " + it.position().viewPosition()) | ||
} | ||
println() | ||
} | ||
|
||
private fun gameWinner(cars: Cars) { | ||
val winners = Winners(cars) | ||
val winnersName = winners.getWinnerNames().joinToString(",") | ||
println(winnersName + "가 최종 우승했습니다.") | ||
OutputView.gameWinner(cars) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,18 +1,10 @@ | ||
package racingcar.domain | ||
|
||
class Car(private var position: Position, private val username: String) { | ||
class Car(var position: Position, val username: 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. position의 setter도 public으로 열리게된 것 같아요! 위 글을 읽어보고 프로퍼티의 setter는 private으로 막아보면 어떨까요? |
||
fun move(number: Int) { | ||
this.position = position.move(number) | ||
} | ||
|
||
fun position(): Position { | ||
return this.position | ||
} | ||
|
||
fun username(): String { | ||
return this.username | ||
} | ||
|
||
companion object { | ||
fun init(username: String): Car { | ||
return Car(Position.init(), username) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,7 +4,7 @@ import racingcar.util.NumberGenerator | |
|
||
private const val USERNAME_SPLIT_CONDITION = "," | ||
|
||
class Cars(private val cars: List<Car>) { | ||
class Cars(val cars: List<Car>) { | ||
fun race(generator: NumberGenerator) { | ||
for (car in cars) { | ||
val number = generator.generate() | ||
|
@@ -15,15 +15,11 @@ class Cars(private val cars: List<Car>) { | |
fun positions(): List<Position> { | ||
val positions: MutableList<Position> = ArrayList() | ||
for (car in cars) { | ||
positions.add(car.position()) | ||
positions.add(car.position) | ||
} | ||
return positions | ||
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. kotlin collection에서 지원하는 map을 이용해서 코드를 조금 더 간결하게 짤 수 있을 것 같아요!! 고민한번 해보면 좋을 것 같습니다 :) 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 cars(): List<Car> { | ||
return cars | ||
} | ||
|
||
companion object { | ||
fun usernames(usernames: String): Cars { | ||
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. 정적 팩토리 메서드 네이밍 컨벤션을 읽어보고, 네이밍에 대해 고민해보면 어떨까요? |
||
return Cars(usernames.split(USERNAME_SPLIT_CONDITION).map { username -> Car.init(username) }) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
package racingcar.view | ||
|
||
object InputView { | ||
fun carNames(): 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. 함수명은 동사형으로 작성해주면 어떨까요? 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. enterCarNames 나 setCarNames 등 할 수 있긴한데, Class 자체가 Input을 하는애들이기 때문에 InputView.carNames() 에 의미가 어느정도 있다고 생각을 하는데. 어떻게 생각하시나요?! |
||
println("경주할 자동차 이름을 입력하세요(이름은 쉼표(,)를 기준으로 구분).") | ||
return readln() | ||
} | ||
|
||
fun roundCount(): Int { | ||
println("시도할 횟수는 몇 회인가요?") | ||
return readln().toInt() | ||
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. 저기서 숫자가 아니면 number format expetion이 터지겠군요. 클라이언트한테 친절한 메시지를 주는게 좋겠네요! |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
package racingcar.view | ||
|
||
import racingcar.domain.Cars | ||
import racingcar.domain.Winners | ||
|
||
object OutputView { | ||
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. outView에 Car객체를 리턴하게 되면 View레벨에서 Car 객체 안에있는 함수를 실행할 수 있을 것 같아요 :) |
||
fun gameResult(cars: Cars) { | ||
cars.cars.forEach { | ||
println(it.username + " : " + it.position.viewPosition()) | ||
} | ||
println() | ||
} | ||
|
||
fun gameWinner(cars: Cars) { | ||
val winners = Winners(cars) | ||
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. Winner를 구하는게 View의 역할일까요? Cars에 있어도 좋을 것 같아요 :) |
||
val winnersName = winners.getWinnerNames().joinToString(",") | ||
println(winnersName + "가 최종 우승했습니다.") | ||
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. kotlin의 String templates을 활용해보면 어떨까요? |
||
} | ||
} |
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.
RandomNumberGenerator는 싱글톤 객체로 만들 수 있지 않을까요?
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.
싱글톤 객체가 필요한 이유가 있을까요?!
메모리적인 이득이나, 다양한 이점이 있을 것 같긴하네요.