Skip to content

[자동차 경주] 김종명 미션 제출합니다.#37

Open
jm991014 wants to merge 15 commits into
woowacourse-precourse:mainfrom
jm991014:jm991014
Open

[자동차 경주] 김종명 미션 제출합니다.#37
jm991014 wants to merge 15 commits into
woowacourse-precourse:mainfrom
jm991014:jm991014

Conversation

@jm991014
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown

@eungyeong12 eungyeong12 left a comment

Choose a reason for hiding this comment

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

2주차 고생 많으셨습니다!
클래스와 함수의 역할이 잘 분리된 것 같고
전체적으로 코드가 깔끔하고 가독성이 좋아 이해하기 쉬웠습니다!

Comment on lines +9 to +14
fun move(randomNumber: Int): Car {
if (randomNumber >= threshold) {
return this.copy(position = position + 1)
}
return this
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

data class의 copy()를 활용한 점 좋네요! 저도 적용해봐야겠어요~

Comment on lines +5 to +7
class RandomNumberGenerator : NumberGenerator {
override fun generateNumber(): Int = Randoms.pickNumberInRange(0, 9)
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

0과 9도 상수화하면 좋을 것 같습니다!

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

다음 과제부터는 꼼꼼하게 적용하겠습니다ㅎㅎ

Comment on lines +5 to +11
class FakeNumberGenerator(
private val numbersToReturn: List<Int>,
) : NumberGenerator {
private var index = 0

override fun generateNumber(): Int = numbersToReturn[index++]
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

테스트용 구현체를 만들어 활용한 점 인상 깊네요!

Comment on lines +17 to +25
fun setup(carNamesInput: String, roundsInput: String) {
val carNames = carNamesInput.split(",")

validateCarNames(carNames)
validateRoundCount(roundsInput)

_cars = carNames.map { name -> Car(name = name) }
_rounds = roundsInput.toInt()
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

입력 파싱과 검증 로직을 외부에서 수행하고, setup에서는 검증된 List<String>Int를 받도록 분리하는 방법도 좋을 것 같습니다!

Copy link
Copy Markdown
Author

@jm991014 jm991014 Oct 29, 2025

Choose a reason for hiding this comment

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

저도 "입력 값에 대한 검증은 누구의 책임일까"에 대해 고민을 좀 해봤습니다.

  1. 입력 값을 받는 View의 책임은 입력을 받는것 까지만인가? 최소한의 검증의 책임을 가지는가?
  2. Controller는 View와 Model을 연결해주는 역할이라면 Controller는 입력값 검증에 대한 책임이 있는가?
  3. 만약 잘못된 입력값이 들어온다면 해당 값을 Model에 넘겨줄 때까지 들고 있는 것이 올바른 구조인가?

아직까지 정확한 답이 나오지는 않았지만 @eungyeong12 님은 어떻게 생각하시나요?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

  1. View의 책임은 입력을 받는 것까지라고 생각해요!

  2. 저는 주로 도메인에서 입력 값 검증을 하고 있습니다. 예를 들어 자동차 이름은 5자를 넘을 수 없고, 시도할 횟수는 1이상의 양수여야 한다는 각자의 규칙이 있기 때문에 각 도메인에서 입력 값에 대한 검증을 수행하는 것이 좋다고 생각해요! 이때 value class를 활용하는 것도 좋은 방법이라고 생각해서 이번에 적용했었습니다.

  3. 빈 문자열이나 정수 변환이 안되는 경우 등 일반적인 입력 오류는 Parser나 Validator에서 미리 처리하는 것도 좋겠다는 생각이 드네요!

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

너무 좋은 방법인 것 같습니다! 저도 3주차 과제에는 적용해봐야겠어요ㅎㅎ 좋은 인사이트 감사합니당

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

좋은 인사이트를 얻어가서 저도 코멘트 달아요! value class에 대해서 알고만 있었지 활용방법은 아직까지 찾지 못했었는데, 원시 타입에 의존하지 않고 비즈니스 규칙을 가진 도메인 객체를 만드는데 사용할수 있겠네요...!🤩

Copy link
Copy Markdown

@angryPodo angryPodo left a comment

Choose a reason for hiding this comment

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

종명님, 2주차 미션 정말 고생 많으셨어요! 😊

MVC 패턴을 적용해서 역할과 책임을 분리하고, 테스트 코드까지 꼼꼼하게 작성하신 것을 보며 정말 깊이 고민하신 흔적을 느낄 수 있었어요. 특히 NumberGenerator 인터페이스를 통해 테스트 용이성을 확보하신 부분은 정말 좋은 설계라고 생각해요.

제가 드린 질문과 의견들이 조금이나마 종명님의 다음 미션에 긍정적인 영향을 준다면 좋겠네요! 저 또한 종명님의 코드를 통해 가변성과 불변성, 객체의 책임에 대해 다시 한번 생각해볼 수 있는 즐거운 시간이었어요. 함께 배우고 성장하는 것 같아 기쁩니다. 👍

다가오는 3주차 미션도 진심으로 응원하겠습니다. 화이팅입니다! 💪🏻

Comment thread README.md
Comment on lines +126 to +129
- [ ] `경주할 자동차 이름을 입력하세요.(이름은 쉼표(,) 기준으로 구분)` 문구를 출력한 후 사용자 입력을 받는다.
- [ ] `시도할 횟수는 몇 회인가요?` 문구를 출력한 후 사용자 입력을 받는다.
- [ ] `실행 결과` 헤더 문구를 출력한다.
- [ ] 각 라운드 종료 후, 모든 자동차의 이름과 현재 위치(`-` 개수)를 형식에 맞춰 출력하고, 라운드 사이에 빈 줄을 추가한다.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

구현 완료된 목록은 표시해주시면 더 좋을 것 같아요~👍🏻

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

헉 깜빡했어요

Comment on lines +13 to +15
private var _rounds: Int = 0
val rounds: Int
get() = _rounds
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

RacingGame_cars_rounds 라는 내부 상태를 직접 변경하시는 방법으로 설계하신 점이 흥미롭네요🤔

각 라운드마다 cars가 변경된 '새로운 RacingGame'을 반환하는 불변 방식과 비교했을떄 현재의 설계를 선택하신 특별한 이유가 있을까요? 객체의 상태를 직접 관리하는 방식이 가지는 장점에 대해 종명님은 어떻게 생각하시는지 궁금해요😊

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

RacingGame은 하나의 경주로서 자동차들의 상태를 가지고 있는것까지가 가져야할 책임의 영역이라 생각해서인데요..!
여기에 추가적으로 라운드의 수나 자동차의 수가 많아질 경우 새로운 자동차를 계속해서 생성하는 것 또한 비용적인 측면에서 비효율적이지 않을까 하는 생각에 해당 방식으로 구현했습니다.

하지만 생각해보니 지금 runOneRound 함수에서 새로운 Car 객체를 할당해주고 있어서 큰 장점이 없는 것 같기도 하네요.. 설계가 좀 부족했던 것 같습니다!

Comment on lines +9 to +14
fun move(randomNumber: Int): Car {
if (randomNumber >= threshold) {
return this.copy(position = position + 1)
}
return this
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Car 모델의 move 함수가 randomNumber를 직접 받아 threshold와 비교하고 있어요. 이렇게 하면 Car가 '4 이상일 때 전진한다'는 구체적인 게임 규칙을 직접 알아야만 한다고 생각이 들어요.

만약 '전진하는 조건' 자체를 MoveStrategy와 같은 인터페이스로 추상화해서 move 함수에 전달한다면, Car는 "전진할지 말지"만 묻게 되어 규칙으로부터 자유로워지고 Car를 더 다양한 규칙의 게임에서 재사용하기 쉬워질 것 같아요.

이 부분에 대해 종명님은 어떻게 생각하시나요? 🧐

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

게임의 규칙도 재사용할 수 있다는 점을 간과한 것 같네요! 확실히 지금대로라면 게임의 규칙이 변경되었을 때 직접 수정해야 하는 단점이 있는 것 같아요.

민재님 의견대로 다음부터 반영해보겠습니다!!

Comment on lines +7 to +8
fun validateCarNames(carNames: List<String>) {
require(carNames.size >= 2) { "자동차는 2대 이상이어야 합니다." }
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

InputValidator에 '자동차는 2대 이상이어야 한다'는 검증 로직을 추가하셨네요! 미션 요구사항에는 없던 부분이라 어떤 고민 끝에 추가하셨는지 궁금해요🤔
1대의 자동차로는 경주의 의미가 없다고 판단하신 걸까요? 종명님만의 규칙을 추가하신 설계 배경을 들려주시면 감사하겠습니다😊

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

맞습니다! 일반적인 경주라 함은 무언가를 겨루는 것이라는 뜻인데, 혼자서 경주를 해봤자 아무 의미가 없다는 생각이 들었어요. 애플리케이션의 목표가 "기록"을 구하는 것이라면 한 대로도 충분할 것 같지만, "경주"가 목적인 이상 최소 두 대 이상은 있어야 하지 않을까? 라는 생각에 추가한 검증로직입니다ㅎㅎ

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

생각하지 못한 부분인데, 최소 2대로 한 부분이 인상적이네요!

Comment on lines +17 to +25
fun setup(carNamesInput: String, roundsInput: String) {
val carNames = carNamesInput.split(",")

validateCarNames(carNames)
validateRoundCount(roundsInput)

_cars = carNames.map { name -> Car(name = name) }
_rounds = roundsInput.toInt()
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

좋은 인사이트를 얻어가서 저도 코멘트 달아요! value class에 대해서 알고만 있었지 활용방법은 아직까지 찾지 못했었는데, 원시 타입에 의존하지 않고 비즈니스 규칙을 가진 도메인 객체를 만드는데 사용할수 있겠네요...!🤩

Comment on lines +8 to +9
class RacingGameExceptionTest {
private lateinit var game: RacingGame
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

RacingGameExceptionTest에서 다양한 예외 케이스를 꼼꼼하게 테스트해주신 점 정말 좋네요! 👍

다만, 현재 여러 개의 테스트 함수로 분리된 유효성 검증 로직들을 JUnit5 @ParameterizedTest@ValueSource 같은 어노테이션을 활용하면, 하나의 함수로 통합하여 더 간결하게 표현할 수 있을 것 같아요. 테스트 코드의 중복을 줄이는 좋은 방법이 될 것 같은데 종명님은 어떻게 생각하시나요?😊

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

저도 이 부분은 이번에 처음 알아서 다음 과제에 한 번 적용해보도록 하겠습니다ㅎㅎ

Comment on lines +28 to +33
companion object {
private const val PROMPT_CAR_NAMES = "경주할 자동차 이름을 입력하세요.(이름은 쉼표(,) 기준으로 구분)"
private const val PROMPT_ROUND_COUNT = "시도할 횟수는 몇 회인가요?"
private const val ROUND_HEADER = "\n실행 결과"
private const val FINAL_WINNER = "최종 우승자 : "
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ConsoleViewcompanion objectFINAL_WINNER = "최종 우승자 : " 와 같이 UI 문자열을 상수로 관리해주신 점이 정말 좋네요!

한 가지 작은 아이디어를 드리자면, 상수를 "최종 우승자"로 정의하고, 출력 시 println("${FINAL_WINNER} : ${...}")처럼 콜론(:)과 공백을 코드에서 조합하는 건 어떨까요? 이렇게 하면 나중에 포맷을 미세하게 변경할 때 문자열 상수 자체를 건드리지 않아도 되어 조금 더 유연해질 수 있을 것 같아요. 사소한 부분이지만 함께 고민해보면 재밌을 것 같아 의견 드립니다! 😄

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

추가로 companion objectprivate 상수로 정의해주셨는데,

저는 보통 이런 파일 전용 상수들은 클래스 밖 최상단에 private const val로 선언하는 편이에요.🙇🏻‍♂️

혹시 companion object 안에 두신 특별한 이유가 있으실까요? 두 방식의 장단점에 대한 종명님의 생각이 궁금합니다. 😄

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

첫 번째 리뷰는 저도 동의합니다! 유연성을 챙기기 너무 좋은 아이디어네요ㅎㅎ

그리고 두 번째 같은 경우에는 깊게 생각해본 적 없는 부분이에요.. 저는 컨벤션의 영역이라 생각합니다. 제가 companion object와 private const val로 선언했을 때 바이트 코드 레벨에서 어떠한 차이가 있는지, 성능상에 차이는 없는지에 대해서는 잘 모르는 것도 있어서 해당 부분에 대해서는 더 알아보고 나중에라도 답변 드리도록 하겠습니다ㅎㅎ

Copy link
Copy Markdown

@hoon4041 hoon4041 left a comment

Choose a reason for hiding this comment

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

고생많으셨어요!!

require(name.isNotBlank()) { "자동차 이름은 공백일 수 없습니다." }
require(name.length <= MAX_NAME_LENGTH) { "자동차 이름은 ${MAX_NAME_LENGTH}자 이하만 가능합니다." }
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

require로 검증을 진행한게 편리하고 좋아보이네요!!

) {
private var _cars: List<Car> = listOf()

private var _rounds: Int = 0
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

cars와 rounds앞에 _를 붙인 이유가 궁금합니다!!

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

컨벤션의 일종이라고 생각해주시면 됩니다! private으로 선언되어 RacingGame 내부에서만 사용되는 변수는 언더바(_)를 붙이고, 외부에서 접근 가능한 변수는 언더바 없이 선언하는 방식으로요!

Copy link
Copy Markdown

@nadajinny nadajinny left a comment

Choose a reason for hiding this comment

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

2주차 과제하느라 고생많으셨습니다! 전반적으로 굉장히 잘 세분화되어있는 코드여서 코드를 코드를 보는데에 굉장히 수월했습니다!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants