[자동차 경주] 정태용 미션 제출합니다.#34
Conversation
- 쉼표(,)로 구분된 이름을 List<String>으로 변환 - 자동차 이름 길이가 6자 이상이면 예외 발생(1~5자만 허용) - 숫자 문자열을 Int로 변환; 비숫자 문자가 포함되면 예외 발생 - IllegalArgumentException 및 에러 메시지 상수 사용
- FixedNumberProvider 구현체 도입 - 4 이상 전진, 3 이하면 정지 검증
- 입력을 Set으로 변환하면서 유일성 검사 수행 - StringToSetConverter 도입 - 중복 시 IllegalArgumentException 발생 (CAR_NAMES_MUST_BE_UNIQUE)
- List 의존 제거, 다양한 컬렉션 수용 - 동작 변화 없음
- 중복 입력 시 IllegalArgumentException 발생 확인 - 메시지: CAR_NAMES_MUST_BE_UNIQUE
joon0447
left a comment
There was a problem hiding this comment.
안녕하세요~ 2주차 과제 고생 많으셨습니다.
인터페이스를 사용하신 것이 인상 깊었습니다!
| interface OutputView { | ||
| fun printCarNamesPrompt() |
There was a problem hiding this comment.
OutputView, InputView를 인터페이스로 선언하신 이유가 궁금합니다!
There was a problem hiding this comment.
OutputView, InputView를 인터페이스로 선언하신 이유가 궁금합니다!
큰 의미는 없습니다!
OutputView 같은 경우는 경주 결과 나 최종우승자를 출력할때 여러 방법이 있을꺼같아서 미리 추상화를 하였던거같습니다!
| try { | ||
| val convertInt = input.toInt() | ||
| if (convertInt <= 0) { | ||
| throw IllegalArgumentException(ATTEMPT_COUNT_MUST_BE_GREATER_THAN_ZERO) | ||
| } | ||
| } catch (e: NumberFormatException) { | ||
| throw IllegalArgumentException(INVALID_ATTEMPT_COUNT_NUMBER) | ||
| } |
There was a problem hiding this comment.
toInt대신 toIntOrNull 을 사용하면 try,catch 문을 없애서 더 깔끔한 코드가 될 것 같네요!
There was a problem hiding this comment.
알려주셔서 감사합니다!
toInt대신 toIntOrNull 을 사용하면 try,catch 문을 없애서 더 깔끔한 코드가 될 것 같네요!
| class StringToSetParser : Parser<Set<String>> { | ||
| override fun parse(input: String): Set<String> { | ||
| val carNamesArr = input.split(",") | ||
| val carNamesSet = HashSet<String>() |
There was a problem hiding this comment.
코틀린에서는 HashSet<String> 보다는 mutableSetOf<String> 이 더 자연스러울 것 같아요!
There was a problem hiding this comment.
코틀린에서는
HashSet<String>보다는mutableSetOf<String>이 더 자연스러울 것 같아요!
알려주셔서 감사합니다! 아직 코틀린 문법에 서투른거같습니다..
| override fun validate(input: String): Unit { | ||
| require(input.contains(",") && input.isNotBlank() && !input.trim().endsWith(',')) { | ||
| ErrorCode.NAMES_MUST_BE_COMMA_SEPARATED | ||
| } | ||
| } |
There was a problem hiding this comment.
조건문이 너무 긴 느낌이 있는 것 같아요.
따로따로 변수에 담아서 작성했으면 더 좋았을 것 같은데 어떻게 생각하시나요??
There was a problem hiding this comment.
조건문이 너무 긴 느낌이 있는 것 같아요. 따로따로 변수에 담아서 작성했으면 더 좋았을 것 같은데 어떻게 생각하시나요??
맞아요 가독성이 떨어지는거같아요
변수에 따로 담아서 작성하는게 더 좋을꺼같아요 감사합니다!
| object ErrorCode { | ||
| const val NAMES_MUST_BE_COMMA_SEPARATED: String = "[ERROR] 이름은 쉼표(,)로 구분해 입력하세요." | ||
| const val INVALID_CAR_NAME_LENGTH: String = "[ERROR] 자동차 이름은 1~5자여야 합니다." | ||
| const val INVALID_ATTEMPT_COUNT_NUMBER: String = "[ERROR] 유효하지 않은 시도 횟수입니다. 숫자만 입력해 주세요." | ||
| const val ATTEMPT_COUNT_MUST_BE_GREATER_THAN_ZERO: String = "[ERROR] 시도 횟수는 0보다 큰 양수여야 합니다." | ||
| const val CAR_NAMES_MUST_BE_UNIQUE = "[ERROR] 자동차 이름은 중복될 수 없습니다." | ||
| const val RACE_MUST_HAVE_PROGRESS = "[ERROR] 모든 자동차가 0칸 이동했습니다. 유효한 경주가 아닙니다." | ||
| } No newline at end of file |
| private lateinit var parser: Parser<Collection<String>> | ||
|
|
||
| @BeforeEach | ||
| fun setUp() { | ||
| parser = StringToListParser() | ||
| } |
There was a problem hiding this comment.
@BeforeEach를 사용하면 더 편하게 테스트를 할 수 있네요!
하나 알아갑니다!!
eungyeong12
left a comment
There was a problem hiding this comment.
2주차 고생 많으셨습니다!
추상클래스나 인터페이스를 잘 활용하셔서 배우고 갑니다!
There was a problem hiding this comment.
자동자 경주 게임 전체의 흐름을 제어하는 역할을 하고 있으니 CarRacingController나 RacingGameController 같은 이름을 사용하면 더 명확할 것 같아요!
There was a problem hiding this comment.
자동자 경주 게임 전체의 흐름을 제어하는 역할을 하고 있으니
CarRacingController나RacingGameController같은 이름을 사용하면 더 명확할 것 같아요!
피드백 감사합니다! 좀더 변수명에 신경써보겠습니다!
| private fun initRacingGame(carNames: Collection<String>, attemptCount: Int) { | ||
| val racingGame = RacingGame(carNames) | ||
| playRacingGame(racingGame, attemptCount) | ||
| } |
There was a problem hiding this comment.
initRacingGame의 파라미터에서 구체적인 타입( List<String>)을 사용하면 더 좋을 것 같습니다! 혹시 Collection을 사용하신 이유가 따로 있을까요?
There was a problem hiding this comment.
initRacingGame의 파라미터에서 구체적인 타입(List<String>)을 사용하면 더 좋을 것 같습니다! 혹시Collection을 사용하신 이유가 따로 있을까요?
Set 타입인 StringToSetParser 구현체 와 List 타입인 StringToListParser 구현체 둘 다 받기 위해 일부러 Collection 인터페이스로 타입을 사용했습니다!
|
|
||
| class RandomNumberProvider : NumberProvider { | ||
| override fun generateNumbers(): Int { | ||
| return Randoms.pickNumberInRange(0, 9) |
There was a problem hiding this comment.
0과 9를 상수화하면 좋을 것 같습니다!
0 , 9 상수까지 신경 못썼네요 피드백 감사합니다!
| class CarTest { | ||
|
|
||
| class FixedNumberProvider(private val n: Int) : NumberProvider { | ||
| override fun generateNumbers(): Int = n | ||
| } | ||
|
|
||
| @Test | ||
| fun `한 라운드에 조건이 만족인 차만 전진`() { | ||
| val racingCar = RacingCar("pobi") | ||
| racingCar.race(FixedNumberProvider(4), DefaultMoveRule()) | ||
| assertEquals(1, racingCar.position) | ||
| } | ||
|
|
||
| @Test | ||
| fun `한 라운드에 조건이 만족하지 않으면 차는 멈춘다`() { | ||
| val racingCar = RacingCar("pobi") | ||
| racingCar.race(FixedNumberProvider(3), DefaultMoveRule()) | ||
| assertEquals(0, racingCar.position) | ||
| } | ||
|
|
||
| } No newline at end of file |
There was a problem hiding this comment.
저도 이 부분에서 테스트를 어떻게 해야 할지 고민이 있었는데, FixedNumberProvider를 구현하여 사용한 점이 인상적이네요!
| val carNamesArr = input.split(",") | ||
| val carNamesSet = HashSet<String>() |
There was a problem hiding this comment.
유지보수 측면에서 변수명에 자료형을 포함하지 않는 carNames, uniqueCarNames 같은 이름을 사용하면 더 좋을 것 같습니다!
There was a problem hiding this comment.
유지보수 측면에서 변수명에 자료형을 포함하지 않는
carNames,uniqueCarNames같은 이름을 사용하면 더 좋을 것 같습니다!
피드백 감사합니다! 변수명에 좀 더 신경써보겠습니다!
| abstract class Car(open val name: String) { | ||
|
|
||
| var position: Int = 0 | ||
| protected set | ||
|
|
||
| fun race(numberProvider: NumberProvider, moveRule: MoveRule) { | ||
| val baseLine = numberProvider.generateNumbers() | ||
| if (moveRule.canMove(baseLine)) move() | ||
| } | ||
|
|
||
| protected abstract fun move() | ||
| } |
There was a problem hiding this comment.
추상클래스를 사용하신 이유가 궁금해요!
요구사항이 바뀌지 않는 이름, 위치 ,race메서드는 재활용 하기위해 상속을 받고자 하였고
요구 사항이 바뀔수 있는 자동차의 전진(지금은 한칸 이동하지만 나중에는 여러칸을 이동할수도 있다고 생각하였습니다. )은 오버라이딩해서 구현하고자 하였습니다!
There was a problem hiding this comment.
확장성을 고려한 설계 좋네요!
좋게 봐주셔서 감사합니다!
| @@ -0,0 +1,5 @@ | |||
| package racingcar.application.parser | |||
|
|
|||
| interface Parser<out O> { | |||
There was a problem hiding this comment.
제너릭 타입으로 인터페이스를 설계하신 부분이 인상적이네요!
가장 잘했던 부분이라고 생각했었는데 알아봐주셔서 감사합니다!
| override fun parse(input: String): Int { | ||
| try { | ||
| val convertInt = input.toInt() | ||
| if (convertInt <= 0) { | ||
| throw IllegalArgumentException(ATTEMPT_COUNT_MUST_BE_GREATER_THAN_ZERO) | ||
| } | ||
| } catch (e: NumberFormatException) { | ||
| throw IllegalArgumentException(INVALID_ATTEMPT_COUNT_NUMBER) | ||
| } | ||
| return input.toInt() |
There was a problem hiding this comment.
현재 convertInt<=0일때 throw를 하고 try-catch로 잡아서 다시 던지는 부분이 조금 비효율적이라는 생각이 듭니다
toInt대신 toIntOrNull을 사용하면 try-catch를 없이도 구현할 수 있을 것 같습니다!
| override fun parse(input: String): Int { | |
| try { | |
| val convertInt = input.toInt() | |
| if (convertInt <= 0) { | |
| throw IllegalArgumentException(ATTEMPT_COUNT_MUST_BE_GREATER_THAN_ZERO) | |
| } | |
| } catch (e: NumberFormatException) { | |
| throw IllegalArgumentException(INVALID_ATTEMPT_COUNT_NUMBER) | |
| } | |
| return input.toInt() | |
| override fun parse(input: String): Int { | |
| val convertInt = input.toIntOrNull() ?: throw IllegalArgumentException(INVALID_ATTEMPT_COUNT_NUMBER) | |
| if (convertInt <= 0) throw IllegalArgumentException(ATTEMPT_COUNT_MUST_BE_GREATER_THAN_ZERO) | |
| return convertInt |
또한 require를 활용하면 아래와 같이 작성할 수도 있을 것 같습니닷
val convertInt = input.toIntOrNull()
require(convertInt != null) { INVALID_ATTEMPT_COUNT_NUMBER }
require(convertInt > 0) { ATTEMPT_COUNT_MUST_BE_GREATER_THAN_ZERO }
There was a problem hiding this comment.
현재
convertInt<=0일때 throw를 하고 try-catch로 잡아서 다시 던지는 부분이 조금 비효율적이라는 생각이 듭니다toInt대신toIntOrNull을 사용하면 try-catch를 없이도 구현할 수 있을 것 같습니다!또한 require를 활용하면 아래와 같이 작성할 수도 있을 것 같습니닷
val convertInt = input.toIntOrNull() require(convertInt != null) { INVALID_ATTEMPT_COUNT_NUMBER } require(convertInt > 0) { ATTEMPT_COUNT_MUST_BE_GREATER_THAN_ZERO }
코드로도 알려주시고 피드백 감사합니다!!
No description provided.