Skip to content

[자동차 경주] 김수대 미션 제출합니다.#52

Open
rlatneorp wants to merge 23 commits intowoowacourse-precourse:mainfrom
rlatneorp:rlatneorp
Open

[자동차 경주] 김수대 미션 제출합니다.#52
rlatneorp wants to merge 23 commits intowoowacourse-precourse:mainfrom
rlatneorp:rlatneorp

Conversation

@rlatneorp
Copy link

No description provided.

Application.kt: 메서드 생성
input을 받아 쉼표로 분류하여 리스트로 만듬
차의 이름을 기능요구 사항에 따르지 않으면 예외 발생
깊이 들여쓰기가 2를 넘기지 않도록 수정
randomNumber(): 랜덤숫자 0~9까지 생성
moveNumber(): 경주 횟수 입력
racingProcessor(): 4이상일 경우 true반환
game(): 차 이름에 랜덤숫자를 배정
 carNameAssignmentStepTime(): game()에서 직관적으로 변경
judgeSingleCar(): 랜덤 숫자가 4일 경우 전진을 함
gameControl(): 레이싱이 입력한 횟수만큼 진행됨
racingBar(): 전진 횟수에 따라 bar사 생성됨
carForRacingJudge(): 레이싱 판정을 위해 호출하도록 변경
judgeSingleCar(): 4이상이여야 전진하는 함수로 수정, depth가 2가 되도록 수정
carNameAssignmentStepTime(): 런타임오류 해결
racingBar(): 반복문으로 barCreator함수를 입력한 힛수만큼 돌림
barCreator(): - 모양을 계속해서 찍어냄
validate 함수들: 매개변수 names를 줘서 중복사용 삭제
validateNamesDelimiter(): 매개변수로 통합
validateNamesCharType(): 불필요한 필터 함수 삭제
timeSetting(): 움직이는 횟수에 예외 수정
judgeFourMoreRole(): random숫자를 입력한 차 숫자만큼 매번 뽑아서 4이상일때만 전진bar 상태를 1더해준다
winner(): 최대로 멀리간 자동차와 최대 거리와 같은 우승자 함수
Copy link

@BaekCCI BaekCCI left a comment

Choose a reason for hiding this comment

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

기능별로 함수를 분리해두셨는데 비슷한 처리를 하는 함수들을 object나 class로 묶어 별도의 파일로 분리하면 더 좋을 것 같습니다!

또한 메서드명을 가능한 동사 기반으로 작성해 함수의 행위를 드러내면 가독성이 더 좋아질 것 같습니다.
예를 들어
fun timeSetting() -> fun setTime(),
fun game() -> fun playGame()과 같이 표현할 수 있습니다!

Comment on lines +91 to +105
fun currentResult() {
for (carName in cars) {
print("$carName : ")
barCreator(carRunStatus[carName])
}
println()
}

fun barCreator(time: Int?) {
val bar = "-"
for (i in 1..time!!) {
print(bar)
}
println()
}
Copy link

Choose a reason for hiding this comment

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

전진 횟수만큼 "-"를 출력할 때 repeat()을 활용하면 코드가 간결해질 것 같습니다!

Suggested change
fun currentResult() {
for (carName in cars) {
print("$carName : ")
barCreator(carRunStatus[carName])
}
println()
}
fun barCreator(time: Int?) {
val bar = "-"
for (i in 1..time!!) {
print(bar)
}
println()
}
fun currentResult() {
for (carName in cars) {
println("$carName : ${"-".repeat(carRunStatus[carName]!!)}")
}
}

Copy link
Author

Choose a reason for hiding this comment

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

일단 리뷰 달아주셔서 정말 감사드린다는 말씀드리고 싶습니다!!
repeat 함수란 게 있었군요! 새롭게 배워가요!
그리고 class랑 파일로 정리해서 코드 작성하는 법에 대해 알아봐야겠어요!
함수명도 동사형으로 쓰는 게 정말 보기에 더 좋고 와닿는 거 같습니다.

정말 감사합니다!!

}

fun stepControl() {
for (i in 1..movingTime) {
Copy link

Choose a reason for hiding this comment

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

for문 대신 repeat을 활용할 수도 있을 것 같습니다!

Suggested change
for (i in 1..movingTime) {
repeat(movingTime) {

Copy link
Author

Choose a reason for hiding this comment

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

우와 간편하네요. repeat...정말 좋은 거 같아요!!

if (!names.all { it.length <= 5 }) {
throw IllegalArgumentException("이름은 5자 이하여야 합니다.")
}
if (!names.all { it.isNotEmpty() }) {
Copy link

Choose a reason for hiding this comment

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

!names.all { it.isNotEmpty() }이 이중 부정(! + isNotEmpty) 형태라
코드를 읽을 때 _"이름이 모두 빈 값이 아닌게 아니라면~"_으로 해석되어 가독성이 조금 떨어질 수 있을 것 같습니다.

동작은 동일하지만 names.any { it.isEmpty() }를 사용하면
"이름이 하나라도 빈 값이라면~" 으로 해석되어 의도가 더 명확하게 드러날 것 같습니다!

Copy link
Author

Choose a reason for hiding this comment

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

아...이런 부분은 생각도 못했어요. all, any 등 가독성에 용이한 활용에 맞게 써야한다고 배워갑니다. 감사합니다!

Copy link

@CommitTheKermit CommitTheKermit left a comment

Choose a reason for hiding this comment

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

코드 잘 읽었습니다. 전체적으로 하나의 파일에 모든 코드가 집중되어 있어서 클래스, 상수 파일로 분리하면 더 좋은 코드가 될 거 같습니다.


fun racingFourMoreRole() {
for (carName in cars) {
judgeFourMoreRole(carName)

Choose a reason for hiding this comment

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

judgeFourMoreRole이라는 함수가 내용이 그리 길지 않아서 따로 함수로 빼기보다 racingFourMoreRole에서 그대로 쓰는 것도 괜찮았을 거 같습니다~

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 +51 to +55
private fun validateNamesCharType(name: String) {
if (!name.all { it.isLetter() }) {
throw IllegalArgumentException("이름은 알파벳만 입력 가능합니다.")
}
}

Choose a reason for hiding this comment

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

해당 함수에는 private이 있는데 다른 함수에는 private이 없네요. 다른 함수에도 붙이시거나 이 함수에서 private을 제거하면 더 통일성 있는 코드가 될 거 같습니다~

Copy link
Author

Choose a reason for hiding this comment

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

맞아요 아직 private 활용도가 이해가 되지 않아, 몇 번 시도를 해봤지만 앞으로는 더 자세히 써야겠다 다짐하게 됩니다.
감사합니다!!

Copy link

@jm991014 jm991014 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 +99 to +105
fun barCreator(time: Int?) {
val bar = "-"
for (i in 1..time!!) {
print(bar)
}
println()
}

Choose a reason for hiding this comment

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

carNameSetting에서 이미 carNames에 대해 기본값으로 0을 넣어주고 있어서 time의 nullable하지 않을 것 같아요.
차라리 carRunStatus를 순회하며 value에 접근하는 방식으로 수정하는건 어떤가요?

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.

4 participants