-
Notifications
You must be signed in to change notification settings - Fork 56
[자동차 경주] 정찬 미션 제출합니다. #41
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
base: main
Are you sure you want to change the base?
Changes from all commits
a80f320
87cae39
258dee4
843d093
88a4c79
8750cbe
4fce448
87b0acd
03df26e
1a9833d
80592fb
a39ed3f
57f4704
e9f4812
36d78ab
fcd5ed6
158bfd2
e4c7017
d976652
d4c3121
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 +1,16 @@ | ||
| # kotlin-racingcar-precourse | ||
|
|
||
| --- | ||
|
|
||
| ## 구현할 기능 목록 | ||
|
|
||
| --- | ||
|
|
||
| - 입력 값을 받는 기능 | ||
| - 입력 값을 검증, 잘못된 값이 올 경우 Exception 하는 기능 | ||
|
Comment on lines
+9
to
+10
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. 구현할 기능을 잘 작성해주셨어요! 개인적으로는 To-Do에 그치지 않고 완료했다는걸 더 명시해주면 좋을 것 같아요🤔 |
||
| - 입력 받은 참가자와 값을 저장해놓는 기능 | ||
| - 무작위 값을 구하는 기능 | ||
| - 무작위 값이 4 이상일 경우 전진하는 기능 | ||
| - 전진 시 출력하는 기능 | ||
| - 랭킹을 정하는 기능 | ||
| - 우승자를 출력하는 기능 | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,17 @@ | ||
| package racingcar | ||
|
|
||
| fun main() { | ||
| // TODO: 프로그램 구현 | ||
| val (inputNames, inputTryCount) = InputView.readInput() | ||
|
|
||
| val names = InputParser.parseNames(inputNames) | ||
| val tryCount = InputParser.parseTryCount(inputTryCount) | ||
|
|
||
| InputValidator.validateName(names) | ||
| InputValidator.validateTryCount(tryCount) | ||
|
|
||
| val cars = names.map { name -> | ||
| Car(name) | ||
| } | ||
|
|
||
| Racing.racingStart(cars, tryCount) | ||
|
Comment on lines
+4
to
+16
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.
만약 main은 전체적인 흐름을 담당하는
Author
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. main함수가 게임의 흐름을 전체적으로 관리하는 함수라고 생각되어 이렇게 구현하였습니다! 그래서 입력을 받고, 검증을 하고, 객체를 생성 후 게임을 진행하는 로직으로 작성했습니다. |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| package racingcar | ||
|
|
||
| class Car( | ||
|
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를 제외한 나머지 객체들을 모두 object로 정의하신 이유가 궁금합니다!
Author
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는 인스턴스를 여러개 생성하지만 나머지는 1개만 생성하고 사용하기 때문에 object로 선엉했습니다! |
||
| val name: String | ||
| ) { | ||
| companion object { | ||
| private const val START_DISTANCE = 0 | ||
| } | ||
|
Comment on lines
+6
to
+8
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. 코틀린 공식 컨벤션 가이드에서 companion object는 클래스 본문의 가장 마지막에 두는 것을 권장하고 있어요!
Author
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 = START_DISTANCE | ||
| private set | ||
|
Comment on lines
+10
to
+11
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의 position을 var, private set으로 관리하고 계시네요! 아마 외부에는 읽기 전용으로 만들기 위해서 사용하셨을 것 같아요. 가변(Mutable)한 설계인데 특별한 이유가 있으실까요? 저는 val로 설계를 했거든요🤔
Author
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의 값이 move를 할 때마다 변하기 때문에 var로 설정하였고, 외부에서 변경을 막기 위해 외부에서는 읽기만, 내부에서는 수정까지 가능하도록 설계했습니다! |
||
|
|
||
| fun moveForward() { | ||
| position++ | ||
| } | ||
| } | ||
|
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. 전체적으로 소스코드에 ⛔ 표시가 되어있는데 혹시 궁금하지 않으셨나요?! 안드로이드 스튜디오 or 인텔리제이에서 lineBreak를 자동으로 추가해주는 기능이 있으니 적용하면 좋을 것 같아요🤗
Author
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. 이야... 정말 감사합니다. 많은 걸 배우는 리뷰네요!! |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| package racingcar | ||
|
|
||
| object InputParser { | ||
| private const val DELIMITER = "," | ||
|
|
||
| fun parseNames(inputNames: String): List<String> { | ||
| return inputNames.split(DELIMITER) | ||
| } | ||
|
Comment on lines
+6
to
+8
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. 이 부분은 저도 고려하지 못했던 부분인데 "pobi , woni"와 같이 이름 앞뒤에 공백을 넣고 입력하는 경우도 있을 것 같아요.
Author
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 parseTryCount(inputTryCount: String): Int { | ||
| val tryCount = inputTryCount.toIntOrNull() ?: throw IllegalArgumentException("시도 횟수는 숫자여야 합니다.") | ||
|
|
||
| return tryCount | ||
| } | ||
|
Comment on lines
+10
to
+14
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. 검증 단계를
Author
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. string -> int로 바꾸는게 파서의 역할이고 그 과정에서 오류가 있다면 Exception을 했던건데 이렇게 봐도 validator로 넘기는게 좋을까요?? 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. 다시 보니 파서에서 처리해도 상관 없을 것 같네요! |
||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,19 @@ | ||||||||||||||||||||||||||||
| package racingcar | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| object InputValidator { | ||||||||||||||||||||||||||||
| private const val MAX_NAME_LENGTH = 5 | ||||||||||||||||||||||||||||
| private const val MIN_TRY_COUNT = 1 | ||||||||||||||||||||||||||||
|
Comment on lines
+4
to
+5
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 validateName(names: List<String>) { | ||||||||||||||||||||||||||||
| names.forEach() { name -> | ||||||||||||||||||||||||||||
| when { | ||||||||||||||||||||||||||||
| name.isEmpty() -> throw IllegalArgumentException("참가자 이름은 비어있을 수 없습니다.") | ||||||||||||||||||||||||||||
| name.length > MAX_NAME_LENGTH -> throw IllegalArgumentException("참가자 이름은 ${MAX_NAME_LENGTH}자 이하여야 합니다.") | ||||||||||||||||||||||||||||
|
Comment on lines
+10
to
+11
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. throw 대신 require를 쓰면 어떨까요??
Suggested change
Author
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. 조언 감사합니다! 더 코틀린스럽고 좋은 것 같습니다! |
||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
Comment on lines
+8
to
+14
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. 반복문을 돌리지 않고 아래처럼 any를 활용해 더 코틀린스럽게 표현할 수도 있을 것 같아요!
Suggested change
Author
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 validateTryCount(tryCount: Int) { | ||||||||||||||||||||||||||||
| if (tryCount < MIN_TRY_COUNT) throw IllegalArgumentException("시도 횟수는 $MIN_TRY_COUNT 이상의 정수여야 합니다.") | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| package racingcar | ||
|
|
||
| import camp.nextstep.edu.missionutils.Console.readLine | ||
|
|
||
| object InputView { | ||
| fun readInput(): Pair<String, String> { | ||
| println("경주할 자동차 이름을 입력하세요.(이름은 쉼표(,) 기준으로 구분)") | ||
|
|
||
| val names = readLine() | ||
|
|
||
| println("시도할 횟수는 몇 회인가요?") | ||
|
|
||
| val tryCount = readLine() | ||
|
|
||
| return (names to tryCount) | ||
| } | ||
|
Comment on lines
+6
to
+16
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. 하나의 함수에서 이름이랑 시도 횟수를 다 입력받고 있군요! |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| package racingcar | ||
|
|
||
| object OutputView { | ||
| fun printRoundResult(cars: List<Car>) { | ||
| cars.forEach { car -> | ||
| println("${car.name} : ${"-".repeat(car.position)}") | ||
| } | ||
| println() | ||
| } | ||
|
|
||
| fun printWinners(winners: List<Car>) { | ||
| val winnerNames = winners.joinToString(", ") { it.name } | ||
| println("최종 우승자 : $winnerNames") | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| package racingcar | ||
|
|
||
| import camp.nextstep.edu.missionutils.Randoms.pickNumberInRange | ||
|
|
||
| object Racing { | ||
| private const val RANDOM_START_NUMBER = 0 | ||
| private const val RANDOM_END_NUMBER = 9 | ||
| private const val MIN_MOVE_VALUE = 4 | ||
|
|
||
| fun racingStart(cars: List<Car>, tryRound: Int) { | ||
| repeat(tryRound) { | ||
| roundStart(cars) | ||
| OutputView.printRoundResult(cars) | ||
| } | ||
|
Comment on lines
+10
to
+14
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.
Author
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. 동의합니다! 전체적으로 고퀄의 리뷰 감사합니다 !! 테스트에 대해 더 생각해보는 계기가 된 것 같습니다! 3주차 과제도 화이팅하세요!! |
||
|
|
||
| val winners = getWinner(cars) | ||
|
|
||
| OutputView.printWinners(winners) | ||
| } | ||
|
|
||
| private fun roundStart(cars: List<Car>) { | ||
| cars.forEach { car -> | ||
| val randomNumber = pickNumberInRange(RANDOM_START_NUMBER, RANDOM_END_NUMBER) | ||
|
|
||
| if (randomNumber >= MIN_MOVE_VALUE) | ||
| car.moveForward() | ||
| } | ||
| } | ||
|
Comment on lines
+21
to
+28
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.
'랜덤 값을 생성하는 로직'을 별도의 인터페이스로 추상화하고 외부에서 주입받는 구조를 적용해보면 어떨까요? 그러면 실제 실행 시에는 랜덤을, 테스트 시에는 항상 4 이상 또는 3 이하의 값을 반환하는 '가짜'를 주입하여, 어떤 상황에서든 예측 가능한 테스트를 작성할 수 있지 않을까 생각이 들어요💪🏻
Author
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 값을 제가 제어할 수 없는 상황이더라구요! @angryPodo 님의 말대로 3주차 과제에 적용할 곳이 있다면 꼭 적용해보겠습니다!! |
||
|
|
||
| private fun getWinner(cars: List<Car>): List<Car> { | ||
| val maxPosition = cars.maxOf { it.position } | ||
|
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. 현재
현재 코드는 후자의 설계를 따르신 것으로 보이는데, 혹시 의도하신 것이 맞을까요? 내부 로직의 예상치 못한 상태에 대해, 이처럼 빠르게 실패하고 문제를 드러내는 것이 더 나은 설계라고 생각하시는지, 아니면 제가 생각한 것처럼 함수가 자체적으로 안정성을 확보하는 것이 더 좋다고 생각하시는지, 정찬님의 의견이 궁금해요😊
Author
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. 저의 의도는 코드상에서 getWinner를 호출하는 시점에 List는 비어있을 수 없다고 생각했습니다. 그래서 따로 가드 코드를 설정하지 않았는데, @angryPodo 님의 말에 대한 의견은 함수 자체적으로 안정성을 확보하는 것이 좋다고 생각합니다! 관점 2보다는 먼저 이런 예외를 생각하고 안정성을 확보하는게 훨씬 좋아보이네요! |
||
|
|
||
| return cars.filter { it.position == maxPosition } | ||
| } | ||
| } | ||
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.
서비스에 대한 소개도 있으면 좋을 것 같아요😊
README 라는 이름 만큼 프로젝트를 파악하는데 가장 처음 보여지는 문서이니 효과적으로 사용하면 더 좋다고 생각해요.
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.
항상 기능만 적어왔는데 다른 분들을 보면 README.md가 잘 되어있더라구요! 저도 이번 피드백과 @angryPodo 님의 피드백을 듣고 조금 더 상세하게 작성해보려고 합니다!! 조언 감사합니다!