[자동차 경주 step1] 김종경 미션 제출합니다. #21
Conversation
|
|
||
| class Car(val name : String, var position : Int = 0){ | ||
| init { | ||
| require(name.length <= 5) { "자동차 이름은 5글자를 초과할 수 없습니다." } |
There was a problem hiding this comment.
require 과 check 의 차이는 무엇일까요?
There was a problem hiding this comment.
공부 해보니 둘 다 조건을 검증하는 데 사용되는 함수라고 합니다.
두 함수 모두 조건을 검증하는 것을 동일하나 사용 목적과 발생시키는 예외 유형이 다르다고 합니다.
require은 함수의 인자 값을 검증할 때 주로 사용되며,
check는 함수 내의 로직에서 상태를 검증할 때 주로 사용한다고 합니다.
자바에는 없던 함수라서 정확히 알지 못하고 사용한 함수인데 덕분에 자세히 공부할 수 있었습니다!
There was a problem hiding this comment.
자바에 없는 함수긴 하지만 illegalArgumentException or
illegalStateException 을 throw 한다는 차이가 있는거라서 두 예외 차이를 공부해보는게 좋을 것 같아요. (자바와 동일합니다.)
| fun move(randomNum : Int){ | ||
| if(randomNum >= 4) position++ | ||
| } |
There was a problem hiding this comment.
자동차가 움직이는 조건을 모르면 훨씬 유연한 자동차를 만들 수 있습니다.
조건을 모르게하려면 어떻게 할 수 있을까요? :)
추가로 4라는 숫자를 다른 개발자가 보면 이해할 수 없을거에요. Magic Number 를 없애고 적절한 이름으로 상수화하는건 어떨까요?
companion object {
private const val CAR_MOVE_CONDITION = 4
}There was a problem hiding this comment.
추가로 Argument randomNum 이 랜덤임을 자동차가 알 필요도 없겠네요!
|
|
||
| import kotlin.random.Random | ||
|
|
||
| class Car(val name : String, var position : Int = 0){ |
There was a problem hiding this comment.
외부에서 position 을 변경할 수 있네요.
자동차의 위치를 캡슐화해서 위치 변경 책임을 자동차에게만 부여하는게 어떤가요?
kotlin 프로퍼티의 getter setter 에 대해 학습해보세요 :)
https://kotlinlang.org/docs/properties.html#getters-and-setters
| fun getRandomNumber(): Int{ | ||
| val numberRange = (MIN_NUMBER_RANGE..MAX_NUMBER_RANGE) | ||
| val num = numberRange.random() | ||
| return num | ||
| } |
There was a problem hiding this comment.
더 코틀린스러운 코드는 어떤가요?
| fun getRandomNumber(): Int{ | |
| val numberRange = (MIN_NUMBER_RANGE..MAX_NUMBER_RANGE) | |
| val num = numberRange.random() | |
| return num | |
| } | |
| fun getRandomNumber: Int = (MIN_NUMBER_RANGE..MAX_NUMBER_RANGE).random() |
| fun inputTryNumber() : Int?{ | ||
| println("시도할 횟수는 몇 회인가요?") | ||
| val input = readLine() | ||
| return try { | ||
| input?.toInt() ?: throw IllegalArgumentException() | ||
| } catch (e: NumberFormatException) { | ||
| throw IllegalArgumentException() | ||
| } | ||
| } |
There was a problem hiding this comment.
| fun inputTryNumber() : Int?{ | |
| println("시도할 횟수는 몇 회인가요?") | |
| val input = readLine() | |
| return try { | |
| input?.toInt() ?: throw IllegalArgumentException() | |
| } catch (e: NumberFormatException) { | |
| throw IllegalArgumentException() | |
| } | |
| } | |
| fun readTryNumber() : Int { | |
| println("시도할 횟수는 몇 회인가요?") | |
| return readln().toIntOrNull() ?: throw IllegalArgumentException() | |
| } |
src/test/kotlin/study/CarTest.kt
Outdated
There was a problem hiding this comment.
테스트 패키지는 구조는 production 코드와 동일하게 가야합니다..!
src/test/kotlin/study/CarTest.kt
Outdated
| @Test | ||
| fun `자동차는 전진 또는 멈출 수 있다`() { | ||
| //Given | ||
| var car1 = Car("김종경") | ||
| car1.move(3) | ||
|
|
||
| var car2 = Car("양두영") | ||
| car2.move(6) | ||
| //Then | ||
| assertThat(car1.position==0 && car2.position==1) | ||
| } |
There was a problem hiding this comment.
요구사항에 맞게 테스트 명을 적는것은 좋아보입니다! 다만 저는 given when then 을 사용할 때 아래와 같이 적는 편이에요.
자동차가 0일 때 전진하면 1 이 된다.
자동차가 3일때 멈추면 3이 된다.
추가로 assertThat() 내부에 로직이 들어가면 직관적이지 못하고 가독성이 떨어질 수 있습니다.
| fun inputCarName(): List<String>? { | ||
| println("경주할 자동차 이름을 입력하세요(이름은 쉼표(,)를 기준으로 구분).") | ||
| val input = readLine() | ||
| val list = input?.split(",") | ||
| return list | ||
| } |
There was a problem hiding this comment.
요것도 코틀린스럽게 간결한 코드가 가능할 것 같아요.
| fun getWinnerList(carList: List<Car>) : List<Car>{ | ||
| val maxDistance = carList.maxOfOrNull { it.position } | ||
| return carList.filter { car -> car.position == maxDistance } | ||
| } |
There was a problem hiding this comment.
변수 이름에 자료구조가 나타나면 자료 구조가 변경될 때 변수 이름을 같이 변경해야합니다..!
getWinners 혹은 judgeWinners 를 사용해볼 수 있을 것 같아요.
There was a problem hiding this comment.
Controller 에 도메인 로직이 많아 보입니다. MVC 에서 비대해진 컨트롤러는 테스트하기 힘들고 재사용성이 떨어집니다. 관심사 분리를 통해 Model 과 View 를 연결해주는 책임만 가지도록 해보는건 어떤가요?
자세한 리뷰 너무 감사합니다! 확실히 mvc에 대해 공부가 조금 더 필요한 느낌이네요ㅠㅠ |
| fun createNewCars(names: List<String>?): List<Car> { | ||
| return names!!.map { name -> Car(name) } | ||
| } |
There was a problem hiding this comment.
newCars 를 만드는데 인자가 nullable 일 필요가 있을까요?
이 객체의 테스트도 빠졌습니다 :(
| if(RandomNumber().getRandomNumber() >= CAR_MOVE_CONDITION) | ||
| car.move() |
There was a problem hiding this comment.
의존하고 있어 테스트하기 힘든 구조인데 어떻게 할 수 있을까요?
No description provided.