Pako 워들제출합니다. #4
Conversation
* 추가된 명세 : hello가 답이고 llloo 라고 입력하면 GRAY, YELLO, GREEN, GRAY, GREEN 로 나와야한다
* 기존에 true, false 로 리턴하는 것을 타일로 변경
- 주석내역 반영
|
|
||
| init { | ||
| require(value.length == WORD_SIZE && regex.matches(value)) { | ||
| throw IllegalArgumentException(WRONG_WORD_SIZE_MESSAGE) |
There was a problem hiding this comment.
require은 이미 내부적으로 IllegalArgumentException를 사용해서 exception을 발생시키도록 되어있어서 message만 정의해주면 될것 같은데 이렇게 한 이유가 있을까요?
| fun `단어는 알파벳이어야한다`() { | ||
| val givenWrongWord = "a1234" | ||
| val message = assertThrows<IllegalArgumentException> { | ||
| Word(givenWrongWord) | ||
| }.message | ||
|
|
||
| assertThat(message).isEqualTo("5글자여야합니다") |
There was a problem hiding this comment.
테스트의 목적(알파벳만 허용)과 단언문에서 확인하는 메세지(5글자)가 서로 동일한 목적으로 보이지 않는것 같습니다!
| @ValueSource(strings = ["abc", "abcdef"]) | ||
| fun `단어는 5글자가 아닌 경우 예외를 던진다`(wrongWord: String) { |
There was a problem hiding this comment.
5글자가 아닌 경우 예외를 확인하고 싶다면 5글자의 경계가 되는 4글자와 6글자가 더 적절할것 같네요!
| YELLOW("\uD83D\uDFE7"), | ||
| GREEN("\uD83D\uDFE9"), | ||
| GRAY("\uD83D\uDFE8"); |
There was a problem hiding this comment.
지금 보여주는 영역(presentation)에서는 노란색, 초록색, 회색으로 결과를 정의하고 있지만 이 부분은 ui변경에 취약하다고 볼 수 있지 않을까요? 도메인 영역이 presentation 영역에 의존적이지 않을까 라는 생각이 드는데 한번쯤 고려해보시고 이를 해결하려면 어떻게 하면 좋은지 고민해보셔도 좋을것 같습니다.
| } | ||
|
|
||
| @Test | ||
| fun `답안과 정답의 한 단어가 위치도 다르고 스펠도 다른 경우 회색이 된다`() { |
| class Answers { | ||
|
|
||
| companion object { | ||
| private const val ANSWERS_TEXT_PATH = "./words.txt" |
There was a problem hiding this comment.
답안은 도메인인데 외부 자원(resource)의 특정 위치를 알고 있어야 할 필요가 있을까요? 도메인 영역은 가능한 독립적으로 구현되어있는게 좋을것 같습니다.
| private fun checkIsWinner(game: Game, resultTiles: Tiles, index: Int) { | ||
| if (game.isWinner(resultTiles)) { | ||
| ResultView.printGamePlayCount(index) | ||
| throw IllegalStateException("게임이 종료되었습니다.") |
There was a problem hiding this comment.
정상적으로 게임이 종료되는 것을 exception으로 구현하는 것은 어색한것 같습니다.
| } | ||
| } | ||
|
|
||
| fun getResourceText(path: String): File { |
| import wordle.domain.Results | ||
| import wordle.domain.Tile | ||
|
|
||
| private const val MAX_TRY_COUNT = 6 |
There was a problem hiding this comment.
Application의 LAST_PLAY_COUNT와 동일한 의미이고 변경도 동일하게 이뤄질것같네요
| fun isWinner(): Boolean { | ||
| val count = tiles.count { it == Tile.GREEN } | ||
|
|
||
| return count == TOTAL_TILE_COUNT | ||
| } | ||
|
|
||
| companion object { | ||
| private const val TOTAL_TILE_COUNT = 5 | ||
| } |
There was a problem hiding this comment.
테스트에서는 모두 GREEN인지를 확인하고 있는데요 그렇다면 그에 맞게 코드도 변경해주는건 어떨까요? 특정 갯수보다 더 명확하게 의미를 나타내도록 코드 수정이 가능할것 같습니다.
준비준비님과 공동 작업물 분리해서 제출합니다.