-
Notifications
You must be signed in to change notification settings - Fork 200
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
[자동차 경주] 배현진 미션 제출합니다. #218
base: main
Are you sure you want to change the base?
Conversation
add files what don't want git to manage. (local.properties, class, log, captures, externalNativeBuild, cxx, apk, output.json, hprof)
write a list of features and requirements in README.md
write basic essential text, num and notation
input car name and split car name
input car name, split car name
input racing execution number
play racing a given number of times entered by the user
input number of play executions
edit Car class, Application file and delete Racing class (racing car move following user input and random range)
move racing car following user input and random range
add Input class file (racing car move following user input and random range)
change FINAL_CHAMPION to FINAL_WINNER
find winner in car racing including duplicates and print winner name using ", "
find winner in car racing including duplicates and print winner name using ", "
create exception file and edit input file by using IllegarArgumentException
delete unnecessary comments and print examples
create exception feature and finish the feature list
delete unnecessary comments
check programming requirements and assignment requirements
create CarRacing file and edit Application file
create test code file
delete fail test code
delete fail test code and add test code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
제가 몰랐던 부분을 알게되어 좋았습니다.👍고생 많으셨고 3주차도 같이 힘내봐요!
import camp.nextstep.edu.missionutils.Randoms | ||
|
||
class CarRacing { | ||
fun startRacingPlay(cars: List<Car>, racing: Int) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CarRacing이라는 클래스 이름으로 의미를 충분히 알 수 있으므로 함수에는 Racing이라는 단어를 빼서 CarRacing.startPlay()
로 호출하면 더 간결하게 표현할 수 있을 것 같습니다. 1주차 공통 피드백에서 '축약하지 않는다' 부분을 다시 한 번 읽어보시면 좋을 것 같아요!
fun startRacingPlay(cars: List<Car>, racing: Int) { | ||
println() | ||
println(Constant.RESULT_TEXT) | ||
for (playNum in 0..<racing) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저도 이렇게 쓰다가 이번에 repeat()
라는 함수를 알게되어서 사용해봤는데 더 편하고 읽기쉽더라구요.
repeat(racingl) {
racingPlay(cars)
println()
}
} | ||
|
||
private fun createPlayRandomNum(): Int = | ||
Randoms.pickNumberInRange(Constant.FORWARD_CONDITION_MIN, Constant.FORWARD_CONDITION_MAX) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
단일 표현식 함수를 중괄호 빼고 표현하니까 더 간결해보이네요. 배워갑니다.👍
import org.junit.jupiter.api.assertThrows | ||
import org.junit.jupiter.api.Test | ||
|
||
class InputTest { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
InputTest
인 만큼 레이싱 횟수 입력에 대한 테스트도 같이 작성해보시는건 어떨까요? 또 자동차가 전진할 때나 우승자가 나올 때, 우승자가 2명 이상일 때 등 작은 단위로 나눠서 테스트를 더 만들어보시면 좋을 것 같습니다.🙌
private fun move(moveCount: Int): String = Constant.FORWARD_NOTATION.repeat(moveCount) | ||
|
||
private fun winnerJudge(cars: List<Car>) { | ||
var moveCountMap: MutableMap<String, Int> = mutableMapOf() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이번 2주차 공통 피드백에서 '변수 이름에 자료형은 사용하지 않는다'라는 피드백이 있었습니다. 사실 저도 이때까지 어겨왔던 규칙이라...3주차에는 변수명을 좀 더 고심해서 짤 예정입니다.
@@ -0,0 +1,19 @@ | |||
package domain | |||
|
|||
class Car(private var name: String) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
name을 가변 변수로 설정하신 이유가 있을까요?👀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
처음에 다른 방식으로 이용해 보려고 시도하던 중 사용했던 것이 그대로 남아있는것 같습니다! 그때그때 상황에 맞춰 코드 짜기 바쁜 모습을 반성하게되네요.. 앞으로 더 신경 써보겠습니다🥲
@@ -0,0 +1,23 @@ | |||
package domain | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
제가 코드리뷰를 깜빡하고 있었네요 죄송합니다. 테스트 코드에 대한 리뷰부터 먼저남길게요. 1주차 해설강의 마지막부분에 테스트코드부분나오는데 그부분 참고하시면 대략적인 감이 잡히실것 같구요, 유튜브 우하한테크코스 채널에 테스트코드 관련 테크톡이 엄청많거든요 그것도 보시면 도움이 많이 되실것같습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
감사합니다! 1주차 해설강의를 보긴했는데 테크톡도 참고해봐야겠네요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2주차 정말 고생많으셨습니다. Exception 처리 등의 코드를 보면서 많이 배울 수 있었습니다. 특히 전체적으로 domain이라는 패키지를 사용하셨는데, MVx 패턴을 적용하시려고 하신게 아닐까생각이 되었습니다. 다만 presentation layer 역할도 domain layer에서 수행하시는것 같아보여 이후에 view까지 분리해보면 더 좋을것 같습니다~~👍👍👍
|
||
fun moveForward(): Int { | ||
moveCount++ | ||
return moveCount |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
결과를 return하신 이유가 있으신건가요???
fun moveStop(): Int { | ||
return moveCount | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moveCount 프로퍼티의 getter를 활용하면 해당 함수가 불필요할것 같아요!!
fun main() { | ||
// TODO: 프로그램 구현 | ||
val carRacing = CarRacing() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
문맥에 따라 코드에 공백을 넣는건 어떨까요?
import camp.nextstep.edu.missionutils.Console | ||
import domain.Constant.NAME_DIVISION_NOTATION | ||
|
||
class Input { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
input이 입력을 받는 곳인줄알았는데 검증을 하는 곳인것 같아서 이름을 Validator등의 검증을 나타내는 단어를 사용하면 어떨까요?
|
||
class Exception { | ||
fun wrongNameException(inputName: String) { | ||
if (inputName.isEmpty()) throw IllegalArgumentException(NO_EXIST_INPUT_EXCEPTION) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
피드백 보셔서 이미 아시겠지만 require함수를 사용하면 좋을것같습니다!
// TODO: 프로그램 구현 | ||
val carRacing = CarRacing() | ||
val input = Input() | ||
println(INPUT_CAR_NAME_TEXT) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ui의 로직분리를 하라고 했으니 ui로직을 담당하는 곳을 따로 만들어서 관리하면 좋을것같아요!
|
||
private fun move(moveCount: Int): String = Constant.FORWARD_NOTATION.repeat(moveCount) | ||
|
||
private fun winnerJudge(cars: List<Car>) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
객체지향 프로그래밍을 저도 잘 이해하지 못하고있어서 틀린 답변일 수도 있으나, 현재 CarRaing의 클래스에서는 경주를 하는 것을 판단되어서 우승자를 가리는 로직은 다른 클래스에서 관리를하는게 어떨까요?
No description provided.