Skip to content
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

[자동차 경주] 황지영 미션 제출합니다. #49

Open
wants to merge 49 commits into
base: main
Choose a base branch
from

Conversation

w36495
Copy link

@w36495 w36495 commented Oct 31, 2023

No description provided.

(1) Message 클래스 내 문구 추가
(2) RacingGameManager 클래스 내 문구 출력 함수 추가
(1) Message 클래스 내 실행 결과 안내 문구 추가
(2) RacingGameManager 클래스 내 실행 결과 안내 출력 함수 추가
(1) Car 클래스 내 랜덤 숫자를 저장할 수 있는 move 변수 생성
(2) Randoms 의 pickNumberInRange(0, 9) 를 통해 랜덤으로 숫자 1개 생성
(3) 각각의 자동차가 가지고 있는 move 변수에 저장
(1) 자동차 전진 메서드 moveForwardCar -> moveCarsForward
(2) 자동자 전진 여부 메서드 isMoveCar -> isCarMovingAllowed
(1) 예외 메세지 추가
(2) Validator 클래스 생성
(3) 사용자로부터 입력을 받아올 때, validator 적용
(1) 파라미터 추가 (input 데이터)
(2) 입력 데이터에 대하여 whitespace 검사 메서드로 분리
(1) 자동차 이름 입력 안내 출력 메소드
(2) 경주 횟수 입력 안내 출력 메소드
(3) 게임 종료 안내 출력 메소드
(1) append() 가 아닌 kotlin 문자열 템플릿 사용
(2) forEach() 가 아닌 repeat 으로 수정
(1) plus() 가 아닌 kotlin 문자열 템플릿 사용
-> 동일한 기능을 inputGameRoundFromUser 가 한다고 생각함
Copy link
Author

@w36495 w36495 left a comment

Choose a reason for hiding this comment

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

이번 미션을 진행하며 읽히기 쉽게 작성하도록 노력하였는데 그렇게 느껴지실지 모르겠습니다 ..
고민이 되었던 부분에 코멘트를 남겨놓았는데 혹시 의견을 가지고 계시다면 코멘트 달아주시면 감사하겠습니다 🥰

모두 고생 많으셨습니다 🔥

}
}

private fun generateRandomNumber(): Int = Randoms.pickNumberInRange(0, 9)
Copy link
Author

Choose a reason for hiding this comment

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

랜덤 숫자를 만드는 부분을 따로 클래스를 만들기에는 가지고 있는 기능이 1개밖에 없는 것 같다고 생각이되어서 클래스를 생성하지 않았는데 이 부분에 대해서 어떻게 생각하시는지 궁금합니다 🥺

Choose a reason for hiding this comment

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

제가 구현할 때는 모든 요구사항들은 변경되거나 추가될 수도 있는 데다, 성격이 다른 메서드라고 생각해서 다른 클래스로 뺐습니다!

Copy link

Choose a reason for hiding this comment

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

랜덤 숫자를 만드는 부분을 따로 클래스로 만드는 이유는,테스트 코드를 통해 라운드 결과를 검증하려는데
랜덤 함수가 숫자 몇을 리턴할지 모르기 때문에 함수 테스트에 어려움이 있습니다.

fun canMoveForward(): Boolean { Random() >= 4}
fun moveForward() { if(canMoveForward()) ...... }
그래서 의존성을 분리해서 테스트에 용이하게 하기 위함입니다!

Choose a reason for hiding this comment

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

저도 지영님처럼 기능이 1개밖에 없다고 생각되어 클래스를 생성하지 않았는데, 다른분들 의견을 들어보니 다음에는 클래스로 분리하여 만드는것도 좋다고 생각됩니다!

Copy link
Author

Choose a reason for hiding this comment

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

@Hevton
오엠지 .. 의존성을 줄이면 테스트가 용이하다 .. 그래서 랜덤 숫자는 따로 분리되어야 하는거군요 ..
새롭게 알아가게 되네요. 감사합니다! 👍

Copy link

@HongYeseul HongYeseul left a comment

Choose a reason for hiding this comment

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

지영님~! 예슬입니다.
저도 다음에는 예외를 개인적으로 생각해보고 구현 해봐야겠어요.
다음 주차때도 파이팅 합쉬다 🔥

Comment on lines +39 to +74
private fun printGameResultOverview(round: Int) {
if (round == 0) {
println()
println(Message.OUTPUT_GAME_END.message)
}
}

private fun printGameResultPerGameRound(cars: List<Car>) {
val stringBuilder = StringBuilder()

cars.forEach { car ->
stringBuilder.append("${car.name} : ")

repeat(car.distance) {
stringBuilder.append("-")
}

println(stringBuilder)
stringBuilder.clear()
}
}

private fun printWinners(winners: List<String>) {
println("${Message.OUTPUT_GAME_WINNER_PREFIX.message}${winners.joinToString()}")
}

private fun inputGameRoundFromUser(): Int {
val gameRound = Console.readLine()

with(InputValidator) {
checkIntType(gameRound)
checkGameRoundSize(gameRound.toInt())
}

return gameRound.toInt()
}

Choose a reason for hiding this comment

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

혹시 print 메서드들을 이곳 안에서 구현하신 이유가 있나요?!

Copy link
Author

@w36495 w36495 Nov 2, 2023

Choose a reason for hiding this comment

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

게임의 모든 것을 관리하도록 RacingGameManager 를 만들었기에 출력도 게임의 일부이기때문에 클래스 안에 두었습니다!
그런데 출력과 관련된 클래스(OutputView 와 같은) 를 따로 만들어야하나 고민이 되었어요 🤔

Comment on lines +27 to +32
fun moveCarsForward(cars: List<Car>): List<Car> {
cars.forEachIndexed { index, _ ->
if (isCarMovingAllowed(generateRandomNumber())) {
increaseCarDistance(index)
}
}

Choose a reason for hiding this comment

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

해당 메서드 이름을 보고 차가 한칸 움직이나보다 생각했었는데 리스트를 받고있길래 보니
랜덤 값을 받아 앞으로 갈 수 있다면 전진하는 메서드네요! 그러한 뜻을 담는 메서드명은 어떨까요?

Copy link
Author

Choose a reason for hiding this comment

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

랜덤 숫자를 파라미터로 받고 해당 메서드에서는 전진 에 포커스를 맞추도록 수정을 해봐야겠네요! 👍

Comment on lines +37 to +40
private fun isCarMovingAllowed(randomNumber: Int): Boolean = when (randomNumber) {
in 4..9 -> true
else -> false
}

Choose a reason for hiding this comment

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

매직 넘버가 사용된것같아요!!

Copy link
Author

Choose a reason for hiding this comment

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

앗 !!!!!!!!! 상수화 시키겠다고 했는데 이 부분은 또 미처 생각을 못했네요 ... 🫣

}
}

private fun generateRandomNumber(): Int = Randoms.pickNumberInRange(0, 9)

Choose a reason for hiding this comment

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

제가 구현할 때는 모든 요구사항들은 변경되거나 추가될 수도 있는 데다, 성격이 다른 메서드라고 생각해서 다른 클래스로 뺐습니다!

Choose a reason for hiding this comment

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

다양한 예외처리가 깔끔하게 정리 돼있어 보기 좋아요 👍

Choose a reason for hiding this comment

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

생각지 못한 다양한 예외 처리가 정리되어있어 보기 좋고 많이 배워갑니다 !!

@@ -1,5 +1,5 @@
package racingcar

fun main() {
// TODO: 프로그램 구현
RacingGameManager().run()

Choose a reason for hiding this comment

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

코틀린은 잘 몰라서 그런데, 코틀린에서는 static 선언을 해주지 않아도 메서드를 바로 호출 할 수 있나요? 😲

Copy link
Author

Choose a reason for hiding this comment

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

앗 그냥 변수 선언없이 바로 객체를 생성한거에요!
자바로 보면 아래와 같습니다ㅎㅎ

RacingGameManager racingGameManager = new RacingGameManager()
racingGameManager.run()

Copy link

Choose a reason for hiding this comment

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

네 객체 생성과 동시에 함수를 호출한 거에요!!

Comment on lines +18 to +22
fun checkInputWhitespace(input: String) {
require(input != "") {
Message.VALID_INPUT_WHITESPACE.message
}
}
Copy link

Choose a reason for hiding this comment

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

checkInputNonBlank()이 공백 검사 기능을 하는 것으로 보이는데 checkInputWhitespace()를 통해 문자열 안의 공백을 허용하지 않으려는 의도이신 걸까요? 공백을 금지하는 것도 좋지만 입력 받는 경우에 trim을 사용하여 입력시 공백을 지워주는 것도 하나의 방법이 될 수 있을 것 같은데 이것에 대해 어떻게 생각하시는지 궁금합니다!

Copy link
Author

Choose a reason for hiding this comment

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

checkInputWhitespace() 는 문자열 공백을 확인하는 메서드로 작성하였습니다!
일단 해당 메서드를 만든 이유는 pobi, ,woni 와 같이 쉼표(,) 사이에 공백만 위치할 경우, 자동차 경주를 할 수 없다고 판단하여 예외가 발생할 수 있도록 만들었습니다.

그렇기 때문에 아래와 같이 checkInputWhitespace() 를 호출하는 부분에서 name.trim() 을 통해 공백을 제거해주었습니다!

    fun validCreateCars(name: String) {
        with(InputValidator) {
            checkCarNameLength(name)
            checkInputWhitespace(name.trim())
        }
    }

그런데 checkInputNonBlank() 에서도 공백검사를 하는데 출력하는 메세지가 다르다보니 메서드가 2개로 나뉜 것 같네요.
메세지를 넘겨줄 수 있도록 리팩토링해봐야겠습니다ㅎㅎ 감사합니다! 👍

}

fun checkCarNameLength(input: String) {
require(input.length in 1..5) {
Copy link

Choose a reason for hiding this comment

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

1주차 공통 피드백을 통해 이름의 의도가 드러난다면 길어도 된다는 말이 있었는데 해당 부분을 길더라도ex)NAME_LENGTH_MINIMUM, NAME_LENGTH_MAXIMUM 로 상수화 할 수 있을 것 같습니다!

Copy link
Author

Choose a reason for hiding this comment

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

앗 헉 이 부분도 상수화를 시키지 못했네요 .. 상수화 시킨게 하나도 없네요 이제보니 .. 😞
감사합니다!

}

fun checkCarMinSize(input: String) {
require(input.split(",").size != 1) {
Copy link

Choose a reason for hiding this comment

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

위와 동일한 내용으로 ex)CAR_SIZE_MINIMUM 로 상수화 할 수 있을 것 같습니다! 또한, 최소 값이 있다면 최대 값도 생성해주는 것을 고민해볼 수 있을 것 같아요!

Copy link
Author

@w36495 w36495 Nov 2, 2023

Choose a reason for hiding this comment

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

최댓값도 10으로 생각은 해두었는데 상수화를 시키지 못했습니다 😞 감사합니다 ..

Comment on lines +36 to +40
fun checkInputPrefix(first: Char) {
require(first != ',') {
Message.VALID_INPUT_PREFIX.message
}
}
Copy link

Choose a reason for hiding this comment

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

저는 첫 문자를 ','로 입력 받는 경우는 생각하지 못했는데 신경을 많이 쓰신 부분인 거 같아요. 배움 주셔서 감사합니다!

require(round in 1..50) {
Message.VALID_ROUND_SIZE.message
}
}
Copy link

Choose a reason for hiding this comment

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

위와 같은 내용으로 ex)ROUND_SIZE_MINIMUM, ROUND_SIZE_MAXIMUM으로 상수화 할 수 있을 것 같습니다!

Copy link
Author

Choose a reason for hiding this comment

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

다음 미션엔 상수화 잊지말아야겠어요 .. 감사합니다!

Copy link

Choose a reason for hiding this comment

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

게임 라운드 입력을 0으로 입력 받았을 때 예외 처리를 할지말지 고민을 많이 했는데 최대 라운드까지 지정하셨네요 제가 놓치고 있던 부분이었던 것 같아요

@zoodung
Copy link

zoodung commented Nov 2, 2023

우선 한 주간 고생 많으셨습니다! 의도하신 방향대로 코드들이 깔끔하게 작성되어 가독성이 매우 좋았습니다. 덕분에 편하게 보고 이해할 수 있었던 거 같아요. 다음 미션도 화이팅입니다!!

Copy link

@Hevton Hevton left a comment

Choose a reason for hiding this comment

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

지영님 이번주차도 함께 해서 즐거웠습니다!
오늘도 지영님께 배운 부분이 많네요. 항상 감사드리며, 저도 도움이 되기 위해 노력하겠습니다.

@@ -1,5 +1,5 @@
package racingcar

fun main() {
// TODO: 프로그램 구현
RacingGameManager().run()
Copy link

Choose a reason for hiding this comment

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

네 객체 생성과 동시에 함수를 호출한 거에요!!

@@ -0,0 +1,20 @@
package racingcar

enum class Message(val message: String) {
Copy link

Choose a reason for hiding this comment

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

Constants object를 사용하지 않고 enum 클래스를 선택하시기까지
어떤 고민이 있으셨는지 여쭤봐도 될까요?

Choose a reason for hiding this comment

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

companion object를 쓸지 enum class를 쓸지 const val을 사용할지에 대해 저도 고민을 많이 했던 부분이라, enum class를 선택하시기까지 어떤 고민이 있었는지 저도 궁금합니다 !

Copy link
Author

@w36495 w36495 Nov 3, 2023

Choose a reason for hiding this comment

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

@Hevton @feijoazoa
헉 .. 사실 Message 클래스를 만들 땐 companion objectconst val 을 떠올리지 못했습니다.
다른 분들의 코드를 봤을 때 enum class 를 통해 입/출력 문구나 에러메세지를 관리하시는 걸 많이 봤는데 그걸 자연스럽게 적용한 것 같네요.
생각지못했던 부분이었는데 짚어주셔서 이유를 생각하지 못했던 조금 부끄럽네요 ..🫣
각각의 차이점을 알아보고 이번 로또미션에 적용해보겠습니다 ..

Comment on lines +18 to +20
carController.run {
createCars(inputCarNameFromUser(Console.readLine()))
}
Copy link

Choose a reason for hiding this comment

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

여기를 run으로 수행하신건, 가독성을 생각해주신건가요?!

Copy link
Author

Choose a reason for hiding this comment

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

run() 을 사용하지 않았을 때, carController.createCars(inputCarNumberFromUser(Console.readLine())) 을 한 줄에 작성하게되면 한 눈에 파악하기 어려울 것 같아 을 사용하게 되었습니다.

Comment on lines +39 to +44
private fun printGameResultOverview(round: Int) {
if (round == 0) {
println()
println(Message.OUTPUT_GAME_END.message)
}
}
Copy link

Choose a reason for hiding this comment

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

handleGameRounds의 repeat 함수 위에 작성해주신다면, 검증 과정이 없어도 괜찮지 않을까 하는 생각이 들었습니당!

Copy link
Author

Choose a reason for hiding this comment

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

헉 그렇게도 되는군요 .. 반복문 바깥으로 빼는 방법은 생각을 못했던 부분이네요 .. 좋은 의견 감사합니다! 👍

Comment on lines +68 to +70
with(InputValidator) {
checkIntType(gameRound)
checkGameRoundSize(gameRound.toInt())
Copy link

Choose a reason for hiding this comment

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

읽기가 편하네요 ㅎㅎㅎ

}
}

fun moveCarsForward(cars: List<Car>): List<Car> {
Copy link

Choose a reason for hiding this comment

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

리턴값으로 List를 두셨는데, 사용되는 곳이 없는 것 같아서 질문 남깁니당!

Copy link
Author

@w36495 w36495 Nov 3, 2023

Choose a reason for hiding this comment

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

그렇네요 ... moveCarsForward() 의 리턴값을 각 라운드의 결과를 출력하는 메서드(printGameResultPerGameRound())의 인자로 사용하도록 작성했는데 바로 getCars() 를 통해 가져오게 작성했네요 .. 😞 짚어주셔서 감사합니다 ..

    printGameResultPerGameRound(carController.getCars())

Comment on lines +68 to +73
with(InputValidator) {
checkInputNonBlank(carName)
checkInputOverSize(carName.split(",").size)
checkCarMinSize(carName)
checkInputPrefix(carName[0])
checkInputPostfix(carName[carName.lastIndex])
Copy link

Choose a reason for hiding this comment

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

이 방식 너무 좋은 것 같아요 배웠습니다!

Choose a reason for hiding this comment

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

with를 사용해서 이렇게 표현할 수 도 있네요! 배워갑니다!


import racingcar.Message

object InputValidator {
Copy link

Choose a reason for hiding this comment

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

예외처리가 너무 멋지네용

}
}

private fun generateRandomNumber(): Int = Randoms.pickNumberInRange(0, 9)
Copy link

Choose a reason for hiding this comment

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

랜덤 숫자를 만드는 부분을 따로 클래스로 만드는 이유는,테스트 코드를 통해 라운드 결과를 검증하려는데
랜덤 함수가 숫자 몇을 리턴할지 모르기 때문에 함수 테스트에 어려움이 있습니다.

fun canMoveForward(): Boolean { Random() >= 4}
fun moveForward() { if(canMoveForward()) ...... }
그래서 의존성을 분리해서 테스트에 용이하게 하기 위함입니다!

Copy link

@sseung416 sseung416 left a comment

Choose a reason for hiding this comment

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

2주차도 너무 수고 많으셨어요! 3주차도 같이 화이팅해봐요!!

Choose a reason for hiding this comment

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

굿굿! 메시지를 모두 상수화하니 보기에도 편하고 좋네요!

Comment on lines +18 to +22
fun checkInputWhitespace(input: String) {
require(input != "") {
Message.VALID_INPUT_WHITESPACE.message
}
}

Choose a reason for hiding this comment

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

코틀린에는 공백을 검증하는 isNotEmpty()isEmpty() 확장 함수도 있어요. 참고하시면 좋을 것 같습니다!

Comment on lines +30 to +46
fun checkCarMinSize(input: String) {
require(input.split(",").size != 1) {
Message.VALID_INPUT_MIN_SIZE.message
}
}

fun checkInputPrefix(first: Char) {
require(first != ',') {
Message.VALID_INPUT_PREFIX.message
}
}

fun checkInputPostfix(last: Char) {
require(last != ',') {
Message.VALID_INPUT_POSTFIX.message
}
}

Choose a reason for hiding this comment

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

separator 값(,)도 상수화하면 좋을 것 같아요.
나중에 separator 값이 바뀌게 된다면, 저 세 값을 모두 바꾸어주어야 하니까요!
그리고 가독성 측면에서도 상수가 더 읽기 편할 것 같아요!

Copy link
Author

Choose a reason for hiding this comment

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

생각지못했던 부분이네요 .. 하지만 말씀주신대로 재사용성측면에서 본다면 상수화되는 것이 좋을 것 같네요! 알려주셔서 감사합니다! 👍

Choose a reason for hiding this comment

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

assertThrows를 사용할 때는 발생한 Exception의 메시지도 같이 검증해야 합니다!
IllegalArgumentException을 상속한 다른 하위 Exception이 발생해도 테스트가 통과되거든요..
저는 1주차 미션 때, 코드를 잘못 작성하여 NumberFormatException이 발생했는데, 이 해당 ExceptionIllegalArgumentException을 상속받기 때문에 테스트가 통과되더라고요.

실제로도 assertThrows의 샘플 코드를 보면 assertEquals와 함께 사용되는 것을 볼 수 있습니다!
image

Copy link
Author

Choose a reason for hiding this comment

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

옴메 .. 사실 이번 미션에 테스트를 공부할 시간이 부족해서 제대로 작성하지 못했어요 😭
다른 분 코드에서도 메세지를 확인하는 분이 계셨는데 이런 부분때문에 그러셨구나 ... 새로 알려주셔서 감사합니다!
이번엔 테스트부분을 파고 들어야 할 것 같습니다ㅎㅎ

Choose a reason for hiding this comment

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

저도 이번 과제에는 예외 처리에 대한 규격을 좀 더 빡빡하게 작성해야 겠어요..
예외에 대한 자세한 내용 작성 굿굿입니다 👍👍


val winners = cars.filter { car ->
car.distance == maxDistanceCar?.distance
}
Copy link

Choose a reason for hiding this comment

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

제가 filter 사용에 익숙치 않았는데 도움이된 것 같습니다!

}

private fun printGameResultPerGameRound(cars: List<Car>) {
val stringBuilder = StringBuilder()

Choose a reason for hiding this comment

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

2주차 공통 피드백에 있었던,, '변수 이름에 자료형,자료구조는 사용하지 않는다'
string을 대신할 네이밍을 하는게 좋아보여요 ! 늘 네이밍 짓기는 너무 어려운것 같습니다ㅜㅜ

Copy link
Author

Choose a reason for hiding this comment

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

정말요ㅠㅠ list 란 단어도 자주 사용해왔는데 이제부터는 지양해야겠어요 .. 신경쓸 것이 너무 많네요ㅠㅠ 알려주셔서 감사합니다! 👍

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.

None yet

7 participants