feat: 준비준비와 파코님과의 과제입니다 #2
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* 추가된 명세 : hello가 답이고 llloo 라고 입력하면 GRAY, YELLO, GREEN, GRAY, GREEN 로 나와야한다
* 기존에 true, false 로 리턴하는 것을 타일로 변경
Flamme1004K
reviewed
Apr 17, 2022
Comment on lines
+5
to
+11
| fun findAnswer(position: Position): Word { | ||
| return words[position.percent(words.size)] | ||
| } | ||
|
|
||
| fun contains(value: String): Boolean { | ||
| return words.contains(Word(value)) | ||
| } |
There was a problem hiding this comment.
Suggested change
| fun findAnswer(position: Position): Word { | |
| return words[position.percent(words.size)] | |
| } | |
| fun contains(value: String): Boolean { | |
| return words.contains(Word(value)) | |
| } | |
| fun findAnswer(position: Position): Word = words[position.percent(words.size)] | |
| fun contains(value: String): Boolean = words.contains(Word(value)) | |
해당 경우에는 문(statement) 보다 간결성을 위하여 식(expression)으로 변경하는 게 좋지 않을까요?
Comment on lines
+2
to
+4
|
|
||
| class Words(private val words: List<Word>) { | ||
|
|
Comment on lines
+2
to
+5
|
|
||
| @JvmInline | ||
| value class Word(val value: String) { | ||
|
|
Comment on lines
+11
to
+13
|
|
||
| private fun isWordSizeAndAlphabet() = value.length == WORD_SIZE && ALPHABET_REGEX.matches(value) | ||
|
|
There was a problem hiding this comment.
Suggested change
| private fun isWordSizeAndAlphabet() = value.length == WORD_SIZE && ALPHABET_REGEX.matches(value) | |
| private fun isWordSizeAndAlphabet() :Boolean = (value.length == WORD_SIZE) && ALPHABET_REGEX.matches(value) | |
- 함수에다가 리턴 타입을 정하지 않고, 타입 추론을 통해서 자동으로 타입을 추론하게 되면 장단점이 무엇이 있을까요?
- 비교에 대한 부분은 그룹을 지어서 표현하는 것이 좋습니다.
Comment on lines
+11
to
+15
|
|
||
| private fun createResultTiles(inputChars: CharArray, wordMatcher: WordMatcher): Tiles { | ||
| return Tiles(inputChars.mapIndexed { index, it -> wordMatcher.match(it.toString(), index) }) | ||
| } | ||
|
|
There was a problem hiding this comment.
글자 하나의 비교를 String으로 비교하는 것이 아닌, Char로 비교해 보는 건 어떠실까요?
Comment on lines
+37
to
+40
| - [ ] 답안과 정답의 단어 중 앞서 한 단어가 판별이 난 경우 뒤에 있는 동일한 단어는 판별되지 않는다 | ||
| - (예 - 정답이 ROYAL인 경우 MOONY를 입력하면 회초회회노 가 결과로 나온다) | ||
| - hello가 답이고 llloo 라고 입력하면 GRAY, YELLO, GREEN, GRAY, GREEN 로 나와야한다 | ||
| - 우선순위가 GREEN -> YELLO -> GRAY 순 |
There was a problem hiding this comment.
가장 중요한 로직에 대한 부분이 아직 구현이 안되었군요! 해당 로직에 대해서 구현해보세요 😊
Comment on lines
+4
to
+6
|
|
||
| object InputView { | ||
|
|
Comment on lines
+8
to
+10
| @Test | ||
| fun `6번의 기회안에 5글자를 모두 맞추면 성공한다`() { | ||
| val givenAnswer = Word("words") |
There was a problem hiding this comment.
'6번의 기회안에 5글자를 모두 맞추면 성공한다' 라는 것보다는 '정답과 답안이 일치하면 게임이 끝난다'가 좋을 것 같습니다!
추가로, 7번 이상 입력했을 때의 테스트 케이스도 만들어 보시는 게 어떠신가요?
Comment on lines
+5
to
+10
| class MockingCreator : Creator { | ||
|
|
||
| override fun createWords(): Words { | ||
| return Words(listOf(Word("queen"), Word("chunk"), Word("awake"))) | ||
| } | ||
| } |
There was a problem hiding this comment.
Words.txt를 읽어들여서 테스트를 하는 게 아닌, 목 데이터를 이용하여 테스트를 하셨군요! 👍👍👍
Comment on lines
+22
to
+26
| val message = assertThrows<IllegalArgumentException> { | ||
| Word(wrongWord) | ||
| }.message | ||
|
|
||
| assertThat(message).isEqualTo("5글자여야합니다") |
There was a problem hiding this comment.
Suggested change
| val message = assertThrows<IllegalArgumentException> { | |
| Word(wrongWord) | |
| }.message | |
| assertThat(message).isEqualTo("5글자여야합니다") | |
| assertThatIllegalArgumentException().isThrownBy { Word(wrongWord) }.withMessageContaining("5글자여야합니다") |
IllegalArgumentException에 대한 테스트에 대해서 이렇게 표현할 수도 있습니다.
|
입출력 요구사항 어플리케이션 실행 결과 현재 입출력 요구사항과 어플리케이션 실행 결과가 다르게 나오고 있습니다. 해당 부분 변경 부탁드립니다! 😊 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.