3단계 - 자동차 경주 #1179
3단계 - 자동차 경주 #1179
Conversation
RacingFieldTest 테스트 코드 작성
경주를 실행할 main 구현 자동차 상태 출력 display 함수 수정
RacingField private 테스트 코드 수정 RacingFieldTest racingFieldGameStartTest 작성
gameCount 만큼 실행 -> 한번만 실행
Car move() 테스트 코드 작성
There was a problem hiding this comment.
태훈님 지난번 리뷰 올리셨을때가 3단계였고 지금은 4단계를 진행하시면 됩니다.
지난 번 3단계때 제가 남긴 부분은 4단계를 진행하기에 앞서 먼저 반영해서 커밋으로 남겨달라는 이야기였습니다. 😅
3단계 리뷰때 제가 남긴 부분에 대해서 잘 반영해주신 부분은 확인했습니다.
하지만 여전히 반영이 되지 않은 부분은 보이는 것 같아 남겨두었습니다.
그리고 4단계를 진행하실때는 요구사항 문서를 먼저 작성해서주시고
그에 따라 TDD 방식으로 실패하는 테스트 -> 성공하도록 코드 수정 -> 코드 개선의 순서를 밟아 코드를 작성하는 것을 부탁드립니다.
TDD로 작성하는 것은 단순히 테스트코드를 작성하기 위함이 아닙니다.
테스트를 작성할 수 있는 코드는 안정적인 설계를 갖춘 코드가 되기 마련입니다.
이전에 태훈님께서 '설계와 리팩토링에 고민을 더 하고싶습니다' 라고 말씀해주셨는데요.
저는 해당 역량을 발전시킬수 있는 방법으로 TDD가 있다고 말씀드리고 싶습니다.
따라서 요구사항 문서를 잘 작성하고 TDD 방식으로 조금씩 코드를 개선해나가면서 연습하시는것을 추천드립니다.
또한 이 과정에서 메세지와 함께 커밋을 잘 남기면 더할 나위 없이 좋을것 같습니다.
지금 요청 주신 PR의 이름은 다음에 요청주실때 이름을 수정해서 4단계로 변경해주시고
제가 리뷰를 남긴 사항, 그리고 지금 본문에서 요청드린 사항, 4단계 요구사항을 모두 충족한 다음 요청주시기 바랍니다!
포기하시지 마시고 끝까지 완주하시기를 바라겠습니다.
| const val DISPLAY_STRING = "-" | ||
| } | ||
|
|
||
| private fun resultMessage() { |
There was a problem hiding this comment.
함수의 이름은 동사로 시작하도록 표현하는 것이 좋습니다!
이 메서드 뿐 아니라 다른 메서드도 모두 고쳐주세요.
https://kotlinlang.org/docs/coding-conventions.html#choose-good-names
There was a problem hiding this comment.
그리고 지난번 리뷰사항 확인 부탁드립니다.
|
|
||
| class ResultView { | ||
|
|
||
| companion object { |
There was a problem hiding this comment.
자바의 static 변수를 맨위에 적는 컨벤션이 있어 헷갈리실수 있을것 같은데
코틀린에서 보통 상수를 표현하기 위해 사용되는 companion object는 클래스의 최하단에 적는 것이 좋습니다.
https://kotlinlang.org/docs/coding-conventions.html#class-layout
| fun getRandomNumber(): Int | ||
| } | ||
|
|
||
| class RandomNumberGenerator : RandomNumber { |
There was a problem hiding this comment.
RandomNumber는 인터페이스의 이름으로 적절해보이지 않습니다.
메서드의 이름으로 미루어보아 조금 더 추상적인 이름인 NumberGenerator는 어떠신가요?
| private val gameCount: Int | ||
| ) { | ||
| constructor(carCount: Int, gameCount: Int) : this( | ||
| List(if (carCount <= 0) 1 else carCount) { Car(randomNumber = RandomNumberGenerator()) }, |
There was a problem hiding this comment.
carCount가 0보다 작으면 1로 취급해야한다는 요구사항은 없는것으로 알고있습니다.
요구사항을 확인하고 다시 작성해주세요.
그리고 if 절을 사용한 뒤에는 중괄호를 반드시 작성해주시기 바랍니다.
다른곳도 마찬가지입니다.
https://kotlinlang.org/docs/coding-conventions.html#control-flow-statements
| if (gameCount <= 0) 1 else gameCount | ||
| ) | ||
|
|
||
| fun getGameCount(): Int { |
There was a problem hiding this comment.
제가 읽기전용 프로퍼티를 이용해서 사용자 정의 게터를 이용하여 표현하는것을 적용해 달라고 말씀드렸는데
이 부분을 비롯해 다른 곳들은 누락하신것 같습니다.
전체적으로 코드를 다시한번 살펴 보시면서 리뷰를 반영하실 수 있는 부분을 찾아보시고 적용해주세요.
| private var distance: Int = 0, | ||
| private val randomNumber: RandomNumber | ||
| ) { | ||
| val readOnlyDistance: Int |
There was a problem hiding this comment.
리뷰드렸던대로 읽기전용 프로퍼티를 활용하여 잘 표현해주셨네요 👍
하지만 변수명이 너무 구체적입니다.
readOnly라는 이름이 val가 아닌 var로 고쳤을때는 알맞은 이름이라고 볼 수 없을것 같습니다.
아쉽게도 IDE는 그러한 부분까지 같이 고쳐주지는 않으니까요.
그러한 맥락에서 변수명이 변수의 특정 행위가 가능한지를 나타내는것은 유지보수를 하고 사용하는데 유리한 이름이 아닌것 같아 보입니다.
다른 이름을 고려해주세요.
| try { | ||
| val value = Integer.parseInt(inputString) | ||
|
|
||
| if (value < 0) { | ||
| println("양수만 입력해 주세요") | ||
| return null | ||
| } |
There was a problem hiding this comment.
4단계 요구사항인 indent 2를 넘어가는 조건을 위배하였습니다.
수정해주세요.
| ) { | ||
| constructor(carCount: Int, gameCount: Int) : this( | ||
| List(if (carCount <= 0) 1 else carCount) { Car(randomNumber = RandomNumberGenerator()) }, | ||
| if (gameCount <= 0) 1 else gameCount |
There was a problem hiding this comment.
이곳도 if 절을 사용한 뒤에는 중괄호를 반드시 작성해주시기 바랍니다.
https://kotlinlang.org/docs/coding-conventions.html#control-flow-statements
|
|
||
| import racingCar.RandomNumber | ||
|
|
||
| class FixedNumberGenerator( |
| } | ||
|
|
||
| @Test | ||
| fun `레이싱 경기 게임 실행 테스트`() { |
There was a problem hiding this comment.
이름이 너무 추상적입니다.
행위에 따른 결과를 나타낼수 있도록 이름을 지어주는 것이 좋습니다.
테스트 이름을 짓는 게 난해하시다면 검색을 해보았을때 도움이 될만한 글들이 많으니 참고해보면 좋을것 같습니다.
예로 what is good name of test code라고 구글에 검색하면 나오는 글입니다.
step3 추가 리뷰 요청 드립니다