[숫자 야구 게임] 짱구 미션 제출합니다. #4
Conversation
| fun announceBaseballGameStartMessage() = println("숫자 야구 게임을 시작합니다.") | ||
|
|
||
| fun announceEnterNumbersMessage() = print("숫자를 입력해주세요 : ") | ||
|
|
||
| fun announceThreeStrike() = println("3개의 숫자를 모두 맞히셨습니다! 게임 종료") | ||
|
|
||
| fun announceBaseballRestartMessage() = println("게임을 새로 시작하려면 1, 종료하려면 2를 입력하세요.") | ||
|
|
||
| fun announceBaseballResult(userInputNumbers: String, randomNumbers: String) = println(calculateBaseballGameResult(userInputNumbers, randomNumbers)) |
There was a problem hiding this comment.
말씀대로 Util 역할을 하는 코틀린 파일에서 최상위 함수로 쓰면 더 좋을 것 같습니다. 지금은 BaseballGame 클래스에 너무 많은 역할이 들어있는 느낌이에용
| fun changeRandomNumbers(number: String): String { | ||
| this.randomNumber = number | ||
| return this.randomNumber!! | ||
| } |
There was a problem hiding this comment.
클래스 내부 상태를 바꾸기보단 불변으로 만들고 매번 새로 만드는게 유지보수하기엔 더 좋을 것 같습니다~
There was a problem hiding this comment.
randomNumber 을 nullable하게 만드신 이유가 따로 있으신가요??
만약 changeRandomNumbers 이 함수가 호출되지 않았는데 다른 함수가 호출되면 NPE가 발생할 것 같아요!
nullable을 사용하고 싶다면 not-null 보다는 ?: 을 쓰는게 더 안전할 것 같아요..!!
제가 옛날에 뭣모르고 !! 남발하다가 NPE 많이 터졌어서 괜히 신경쓰게 되네요 ㅎㅎ ㅠㅠ
| """.trimIndent()) } | ||
| ?.apply { Console.readLine() | ||
| .takeIf { it == "2" } | ||
| ?.apply { exitProcess(0) } } |
There was a problem hiding this comment.
exitProcess는 처음 봤네요 ㅎㅎ
학습용으로 메서드 체이닝을 사용하신건 좋습니다만 오히려 무분별하게 사용해서 가독성이 더 떨어지는 경우도 많습니다. 워낙 코틀린이 함수형 지원을 잘해줘서 편하다보니 그렇게 하는데, 실무에서도 되게 지양하는 방향이라 중간 지점을 잘 찾아서 사용하면 더 좋을 것 같아요~
There was a problem hiding this comment.
주석 스텝별로 돼있어서 너무 보기 편하네요 so good 👍👍
| fun announceBaseballGameStart() = println("숫자 야구 게임을 시작합니다.") | ||
|
|
||
| fun printReceiveNumberMessage() = print("숫자를 입력해주세요 : ") | ||
| fun announceGameResult(baseballGameResult: BaseballResultDto) = |
There was a problem hiding this comment.
메서드 개행 기준도 세워서 나눠주면 더 좋을 것 같아요
|
깊은 고민을 하시며 공부하신 티가 팍팍나네요~ 짱. 궁금했던 점에 올려주신 사항들 저도 고민했던 부분들인지라 이슈에 등록해서 코드 리뷰와 논의를 분리하면 좋겠다는 생각이 들었습니다!! 이슈 등록해주시면 감사하겠습니다 ㅎㅎ |
| } | ||
|
|
||
| fun getBaseballGameResult(userInputNumbers: String): BaseballResultDto = | ||
| requireNotNull(userInputNumbers.takeIf { it.length == 3 && it.toIntOrNull() != null }) { |
There was a problem hiding this comment.
이거 몇몇 선배님들께서 상수,Enum로 관리하시던데 그렇게 바꿔보는 것도 좋을 것 같습니다!! (저도 바꿀 예정..헷)
There was a problem hiding this comment.
그리고 required 쓰는거 좋은 것 같습니다!
| fun countStrike(number: String): Int = number.indices.count { i -> number[i] == randomNumber!![i] } | ||
|
|
||
| fun countBall(number: String): Int = number.indices.count { i -> number[i] != randomNumber!![i] && number[i] in randomNumber!! } | ||
| } |
There was a problem hiding this comment.
저는 이거 비교하는 역할을 service나 아니면 baseball controller와 같이 다른 곳에 뒀는데 random numbers클래스 안에 두고 개수를 세는게 더 좋은지 궁금합니다!
service에서 간편하게 비교하기 위해서 사용하셨나요??
| generateSequence { Randoms.pickNumberInRange(1, 9) } | ||
| .distinct() | ||
| .take(count) | ||
| .joinToString("") |
There was a problem hiding this comment.
오 저는 set으로 사용했는데 distinct()를 사용하면 되군요!!
그리고 숫자를 String으로 받으셨군요!
There was a problem hiding this comment.
우왕... 굳이네요!! 👍
그런데 @stopmin님이 말씀하신 것 처럼 메서드 명은 createUniqueRandomNumbers()인데 반환형은 String이라서 이 함수를 사용하는 사람이 혼란이 올 수도 있을 것 같습니다~!
| return randomNumbers.joinToString(separator = "") | ||
| } | ||
|
|
||
| fun isUserInputValid(userInputNumbers: String): Boolean = userInputNumbers.length == 3 && userInputNumbers.toIntOrNull() != null |
There was a problem hiding this comment.
사용자가 0을 입력한 경우도 Invalid하다고 판단해줘야 하지 않을까요?
|
|
||
|
|
||
| fun String.isThreeStrike(numbers: String, randomNumbers: String): Boolean = | ||
| numbers[0] == randomNumbers[0] && numbers[1] == randomNumbers[1] && numbers[2] == randomNumbers[2] |
There was a problem hiding this comment.
이 부분은 간단하게 numbers == randomNumbers로 바꿔도 좋을 것 같아요!
| fun changeRandomNumbers(number: String): String { | ||
| this.randomNumber = number | ||
| return this.randomNumber!! | ||
| } |
There was a problem hiding this comment.
randomNumber 을 nullable하게 만드신 이유가 따로 있으신가요??
만약 changeRandomNumbers 이 함수가 호출되지 않았는데 다른 함수가 호출되면 NPE가 발생할 것 같아요!
nullable을 사용하고 싶다면 not-null 보다는 ?: 을 쓰는게 더 안전할 것 같아요..!!
제가 옛날에 뭣모르고 !! 남발하다가 NPE 많이 터졌어서 괜히 신경쓰게 되네요 ㅎㅎ ㅠㅠ
| """.trimIndent()) } | ||
| ?.apply { Console.readLine() | ||
| .takeIf { it == "2" } | ||
| ?.apply { exitProcess(0) } } |
There was a problem hiding this comment.
주석 스텝별로 돼있어서 너무 보기 편하네요 so good 👍👍
| import kotlin.system.exitProcess | ||
|
|
||
| class BaseballApplication( | ||
| val baseballView: BaseballView, |
There was a problem hiding this comment.
property는 보통 private을 선언해줍니다! 참고자료
There was a problem hiding this comment.
구현하시느라 수고 많으셨습니다~~! 처음 보는게 많아서 리뷰하면서 신기했습니다 ㅎㅎ
다음 미션부터 리플렉션을 사용 해 봐도 될까요?
학습 차원에서 한번 사용 해 보고 싶습니다! 미션 제한 사항에 외부 라이브러리는 사용하지 말라고 되어 있어서 여쭤봅니다! 학습 차원에서 리플렉션 끄적여 보고 싶네요..!!
음! 저희끼리 진행하는 스터디니까 학습용도로 사용해봐도 좋다고 생각합니다~~
프리코스 요구사항에 안내되어 있는 규칙들을 얼만큼 따를 건지 정해보면 좋겠네요! 이슈에 등록해놓겠습니다~
| fun createBaseball() { | ||
| val createUniqueRandomNumbers = createUniqueRandomNumbers(3) | ||
| RandomNumbers.changeRandomNumbers(createUniqueRandomNumbers) | ||
| } |
There was a problem hiding this comment.
createBaseball() 메서드를 통해 랜덤한 야구 게임 숫자를 생성해준다는 걸 메서드 명으로 더 표현하면 좋을 것 같습니다~
| generateSequence { Randoms.pickNumberInRange(1, 9) } | ||
| .distinct() | ||
| .take(count) | ||
| .joinToString("") |
There was a problem hiding this comment.
우왕... 굳이네요!! 👍
그런데 @stopmin님이 말씀하신 것 처럼 메서드 명은 createUniqueRandomNumbers()인데 반환형은 String이라서 이 함수를 사용하는 사람이 혼란이 올 수도 있을 것 같습니다~!
| @@ -0,0 +1,14 @@ | |||
| package step2.entity | |||
|
|
|||
| object RandomNumbers { | |||
There was a problem hiding this comment.
RandomNumbers를 object로 선언하셨네요!!
찾아보니 싱글톤을 보장해주는 것 같은데, 아직 잘 모르겠어서 이렇게 정의하신 이유가 궁금해요!
| .take(count) | ||
| .joinToString("") | ||
|
|
||
| fun getThreeUniqueNumbers(): String = Console.readLine() ?: throw IllegalArgumentException() |
There was a problem hiding this comment.
함수명은 getThreeUniqueNumbers()인데 함수에서는 그저 Console로 값을 입력받고, 그것이 null이라면 예외를 반환하는 거라서 함수 명과 실제 동작이 다른 것 같아요!
| data class BaseballResultDto( | ||
| val strikeCount: Int, | ||
| val ballCount: Int, | ||
| ) |
There was a problem hiding this comment.
혹시 포매터 어떤 거 쓰시나요??
코틀린은 인텔리제이가 자동으로 포매팅을 지원해준다고 알고 있는데
탭 간격이 (우테코 자바 포매터가 적용된건지ㅠㅠ) 자꾸 바뀌네요...😵💫 왜이럴까요
| fun getBaseballGameResult(userInputNumbers: String): BaseballResultDto = | ||
| requireNotNull(userInputNumbers.takeIf { it.length == 3 && it.toIntOrNull() != null }) { | ||
| "유효하지 않은 숫자 또는 문자입니다." | ||
| }.let { baseballService.compareRandomNumbers(it) } |
| "유효하지 않은 숫자 또는 문자입니다." | ||
| }.let { baseballService.compareRandomNumbers(it) } | ||
|
|
||
| } |
| with(baseballGameResult) { | ||
| listOfNotNull( | ||
| ballCount.takeIf { it > 0 }?.let { "${it}볼" }, | ||
| strikeCount.takeIf { it > 0 }?.let { "${it}스트라이크" } | ||
| ).takeIf { it.isNotEmpty() }?.joinToString(" ") ?: "낫싱" | ||
| } |
There was a problem hiding this comment.
우와... 뭔지 모르겠는게 많아서 5줄 읽는 데 한참 걸린 것 같아요 ㅋㅋ🤣👍
인삿말
한 분 빼고 모두 우테코 프리코스 스터디로 또 뵙네요~ @1jeongg 님도 반갑습니다~ 코틀린 고수 ㄷㄷ..!!
코틀린 너무 낯설어서 코드 작성하는데 시간이 오래 걸렸네요.. 코드 짜면서 다른 분들 어떻게 작성하셨을까 정말 궁금했었고, 다른 분들 생각도 너무 궁금했습니다..ㅎㅎ 잘 부탁드리겠습니다
+) 파일을 많이 생성했는데 src/main/kotlin/step2 만 보시면 될 것 같습니다!! 다른 파일들은 책 읽으면서 끄적였던 것들 입니다..ㅎㅎ
궁금했던 점
코틀린 전망?
엄청난 회사에서 일 하시는 분도 계시고, 인턴으로 일 하시는 분도 계시고, 학생 분도 계시는데 직무에서 코틀린이 많이 쓰이고 있는지 또, 전망이 좋은지 여러 관점에서 의견을 들어보고 싶었습니다..! ㅎㅎ
대학교 교수님께서는 코틀린 접고 자바 21이나 하라는 충격적인 말을 들어서 조금 걱정스러웠습니다..ㅠ
최상위 함수 vs 유틸 클래스
코틀린은 자바와 달리 두 가지 선택지를 제공하는데 어떤 것을 선택해야 할 지 잘 모르겠습니다.
기존은 RedisUtil.setDataExpired 처럼 사용해서 redis의 데이터를 만료하는 동작을 예상할 수 있는데 최상위 함수로 선언하면 데이터를 만료하는데 어떤 데이터를 만료 하는지 잘 모를 것 같습니다. 물론 더 의미있는 이름으로 작성 하겠지만 어떤 방법이 더 좋을지 감을 못 잡겠습니다.. 다른 분들의 의견도 궁금합니다.
리플렉션 사용 여부
다음 미션부터 리플렉션을 사용 해 봐도 될까요?
학습 차원에서 한번 사용 해 보고 싶습니다! 미션 제한 사항에 외부 라이브러리는 사용하지 말라고 되어 있어서 여쭤봅니다! 학습 차원에서 리플렉션 끄적여 보고 싶네요..!!
자랑하고 싶은 점 및 죄송한 점
자랑하고 싶은 점
죄송한 점
테스트를 못 짯습니다.. 죄송합니다.. 하지만 저도 최첨단 수동 인간 테스트는 완료했기 때문에 걱정 하지 않으셔도 됩니다!!!