[Wordle] 호우팀(우유) 미션 제출합니다. #19

woo-yu · 2024-06-16T15:13:39Z

호이스토리님 짱!

- 단어가 존재하지 않을 경우 - 단어가 존재 할 경우 - 현재날짜 기준으로 단어 추출

- 스테이지 상태가 fail - 스테이지 상태가 progress일때 - 정답을 맞췄을때 - 정답을 맞추지 못했을때

- 완전히 동일한 단어리 경우 - 포함되어있지만 위치가 일치하지 않을 경우 - 단어 포함, 위치 일치 - 단어는 포함되어 있으나 위치가 일치하지 않고 정답개수의 단어보다 입력 갯수가 더 많을 경우 일치한 횟수(왼쪽에서 오른쪽으로 계산)가 적을 경우 - 단어는 포함조차 안되어있을 경우

- 모두 영문으로 구성 - 5글자가 아닐 경우 - 영어를 대문자에서 소문자로 치환 - 단어가 존재하지 않을 경우

- 파일에서 특정 단어 추출 - 단어 존재여부 체크

- 게임 시작

- 단어 입력

- 게임 시작 문구 - 정답 입력 요청 문구 - 결과 문구

- 게임 진행 여부 - 게임 플레이

- 단계별 답 체크

- 단어 5글자 여부 체크 - 단어 영문 여부 체크 - 존재 야부 체크 - 소문자로 치환

kwj1270 · 2024-06-23T05:42:53Z

build.gradle.kts

@@ -7,7 +7,7 @@ group = "camp.nextstep.edu"
 version = "1.0-SNAPSHOT"

 kotlin {
-    jvmToolchain(21)
+    jvmToolchain(17)


gradle 버전 문제로 인한 변경 확인했습니다 👍🏻

kwj1270

안녕하세요 우유님, 이번 과제 리뷰어를 맡게된 김우재입니다 😃
예외 케이스와 기능 구현도 완벽하게 작성해주셨던 점이 매우 좋았고
전체적으로, 단순하고 직관적인 코드들로 인해 이해가 매우 편했던 것 같습니다.
(보면서 이렇게 쉽게 만들 수 있구나 감탄도 했습니다.)

리뷰 관련해서 몇가지 제 개인적인 생각을 남겼는데,
확인해주시면 우유님 의견도 남겨주시면 감사드리겠습니다 :D

kwj1270 · 2024-06-23T05:56:00Z

src/main/kotlin/wordle/Dictionary.kt

+    fun findTodayWord(nowDate: LocalDate): String {
+        val index = nowDate.toEpochDay().minus(BASE_DATE.toEpochDay()).toInt() % words.size
+        return words[index]
+    }


정답은 매일 바뀌며 ((현재 날짜 - 2021년 6월 19일) % 배열의 크기) 번째의 단어이다. 에 대한 요구사항 만족 좋습니다 👍🏻
다만, 정답 선택의 기준이 바뀐다면 여러 클래스들에 있어 수정이 필요해질 것으로 보여집니다.

LocalDate 를 사용하는 Application,Game, Dictionary 세 부분의 코드가 수정될 것 같고
메서드 네이밍도 findTodayWord 에서 다른 이름으로 변경될 여지가 있어보입니다.
결합도가 높아서 발생하는 것이라고 생각이 드는데 단어 선택 방식 을 추상화해보면 어떨까요?

단어 선택 방식에 대해서 구현체마다 알맞게 알고리즘을 구현해서 선택할 수 있을 것 같으며
응집성이 높아져 변경 전파도 최소화될 수 있을 것 같습니다 :D

오 좋은 의견 주셔서 감사합니다 👍👍
단어 선택 방식 이라는 이름의 AnswerSelector를 만들어보았습니다. ㅎ_ㅎ

저의 경우 AnswerSelector 로 만들지 WordSelector 로 만들지에 대한 고민이 많았어요
우유님은 어떻게 풀어나가실지 매우 궁금하네요 :D

kwj1270 · 2024-06-23T06:13:33Z

src/main/kotlin/wordle/Dictionary.kt

+    private const val PATH = "./src/main/resources"
+    private const val FILE_NAME = "words.txt"
+    private val words: List<String> = File(PATH, FILE_NAME).readLines()


PATH 와 FILE_NAME 을 분리하고 이를 통해 Words 를 가져오신 아이디어 매우 좋다고 생각합니다. 👍🏻
다만, 조금 아쉬운 점은 File 에 대한 영구적인 의존성을 가지고 있는 것에 대해서 역할과 책임이 크다는 생각이 들었습니다.

단어가 파일 시스템이 아닌 DB 를 이용하는 것으로 바뀐다면 아래와 같은 방법으로 대응을 해야할 것 같습니다.

PATH, FILE_NAME 을 지우고 Dictionary 에서 DB 를 조회하는 형태로 수정한다.

PATH, FILE_NAME 을 지우고 외부에서 words 를 주입받는 형태로 수정한다.

Dictionary 를 만들기 위한 데이터 조회 부분이 바뀌는 것인데 Dictionary 가 수정되는게 다소 어색하다고 느껴지네요.
Dictionary 에서 단어를 읽는 부분을 분리하는 방향은 어떨까요?
파일 시스템이 아닌 다른 시스템으로 변경을 하더라도 Dictionary 에는 영향을 주지 않을 것 같은데
우유님 의견이 궁금합니다 :D

생각해볼만한 좋은 주제 던져주셔서 감사합니다 👍
우지님 덕분에 재밌는 고민을 많이 해볼 수 있는 것 같아요!

Dictionary는 단어 목록을 관리하는 추상 역할만 수행하면 된다고 생각하고,
구현체에 따라 단어목록을 조회해오는지 여부를 나눠보도록했습니다. 그래서 FileDictionary 라는 형태로 구현하도록 해보았습니다. 😄

Dictionary는 단어 목록을 관리하는 추상 역할만 수행하면 된다. 에 대해서 공감합니다. 👍🏻
현재 요구사항 기점으로는 FileDictionary 정도로만 사용해도 충분할 것 같습니다.

kwj1270 · 2024-06-23T07:10:20Z

src/main/kotlin/wordle/Game.kt

+                require(Dictionary.hasWord(word)) { "존재하지 않는 단어입니다." }
+                return word
+            } catch (e: Exception) {
+                println(e.message)


다른 코드들은 Output 클래스를 활용하는데 println 을 쓰신 이유가 있으신가요?
Output 을 활용하여 출력에 관한 로직을 응집하게 몰아 넣을 수 있을 것 같습니다

오... 안그래도 호아님코드 리뷰하면서 고쳐야지 생각했는데 잘 짚어주셨군요 👍👍👍

kwj1270 · 2024-06-23T07:12:16Z

src/main/kotlin/wordle/domain/Word.kt

+/**
+ * 사용자의 검증된 입력 단어
+ */
+@JvmInline
+value class Word(val value: String) {
+
+    companion object {
+        private val englishRegex = Regex("^[A-Za-z]*")
+
+        fun fromInput(input: String): Word {
+            require(input.matches(englishRegex)) { "영문만 입력해야합니다." }
+            require(input.length == 5) { "5글자여야 합니다." }
+            return Word(input.lowercase())
+        }
+    }
+
+}


validation 체크에 대해서 잘 보았습니다. 👍🏻
다만 다른 사람이 코드를 작성했을 때 생성자를 활용할 수 있다는 생각이 드는데요.
validation 관련 로직을 일반 생성자에서도 처리할 수 있게끔 구조를 변경하는 것에 대해서는 어떻게 생각하시나요?

요건 코틀린에서는 data class/value class의 생성자 제약사항이 많아서 고민을 많이 했었는데요... 🤔

private constructor에는 반드시 val/var property 포함

현재의 요구사항으로는 입력받은 값을 lowercase()로 변경해줘야함

여러번 시도하다가 static method로 분리하는걸 선택했는데,
다시 도전해보다가 secondary constructor를 이용하는 방법을 사용해보았습니다..?! 🥹
뭐가 정답일지는 항상 고민이 되는군요. ㄷㄷㄷ

저 같은 경우 Kotlin 를 사용할 때 객체 생성시 검증은 주로 init 블록을 활용하고 있어요.
단, 호출 시점이 헷갈리는 경우가 많은데 관련해서 링크 하나 첨부할게요 :D

kwj1270 · 2024-06-23T07:13:27Z

src/main/kotlin/wordle/view/Output.kt

+    private fun showStep(step: Step) {
+        println(step.result.joinToString("") {
+            when (it) {
+                Step.Result.CORRECT -> "\uD83D\uDFE9"
+                Step.Result.MISMATCH -> "\uD83D\uDFE8"
+                Step.Result.WRONG -> "⬜"
+            }
+        })
+    }


UI에 표현되는 색상과, 단어 매칭 결과에 대해서 나눈점 매우 좋은 것 같습니다 👍🏻

kwj1270 · 2024-06-23T07:30:33Z

src/main/kotlin/wordle/view/Input.kt

+        return Word.fromInput(readln().trim())
+    }
+
+}


GitHub 에서는 EOF 에 대해서 경고 메시지를 보내고 있습니다.
관련해서 링크 하나 첨부 드리는데 내용 확인해보시면 좋을 것 같습니다 :D

오 감사합니다!!
회사에선 editorconfig 설정 외엔 모든 IDE 설정을 제외하도록 해놨더니 요런것도 빼먹고 있었군요 ㄷㄷ

kwj1270 · 2024-06-23T07:31:54Z

src/test/kotlin/wordle/DictionaryTest.kt

+    @ParameterizedTest
+    @ValueSource(strings = ["asder", "wrfdx", "bljwq"])
+    fun `단어가 존재하지 않으면 false`(word: String) {
+        //when
+        val result = Dictionary.hasWord(Word(word))
+
+        //then
+        assertThat(result).isFalse()
+    }


단어장에 대한 예외처리 너무 좋습니다 👍🏻
다만, 2자기 아쉬운 부분이 존재하는 것 같습니다.

단어장에 어떤 단어들이 있는지 명시적으로 보이지가 않는다.

"./src/main/resources/words.txt" 파일이 수정되면 테스트가 실패한다.

관련해서 어떤식으로 테스트 및 설계를 변경하면 좋을지 고민해보면 좋을 것 같습니다!

오 감사합니다 👍
위에서 Dictionary와 FileDictionary로 구분하면서 Mock으로 선언해서 1번의 내용은 해소되었군요! ㅋㅋㅋ

2번의 내용은 역으로 질문드리고 싶은데요.
저 파일의 경로가 만일 DB라고 가정한다면, datasource 경로가 잘못되었을때의 테스트도 하는 편이신가요? 🙇‍♀️
이미 인프라적인 요소로 넘어가서 테스트 대상에서 예외가 된거같긴하지만 (도메인쪽만 테스트필수)
경로에 대해서도 고민하는 편이신지 궁금합니닷 😄

상당히 흥미로운 주제인 것 같네요 👍🏻
datasource 경로가 잘못되었을 때의 테스트 하는 편인가? 란 질문에 대해서만 답변 드리자면 저는 테스트하지 않을 것 같아요.
실무 관점에서, 로컬/개발/운영 환경이 나누어져있기도 하고 이를 테스트하는 것은 매우 어렵다는 생각이 듭니다.(권한 문제 등등)
그리고 DataSource 의 경우 Lazy 설정이 아니라면, 빌드 시점 혹은 배포 초기에 바로 파악이 가능할 것 같다는 생각도 들고요.
그리고 정말, Datasource 의 경로가 잘못되었다면, 코드 리뷰등을 통해서 팀 차원에서도 검증 할 수 있지 않을까 생각해요

kwj1270 · 2024-06-23T07:36:35Z

src/test/kotlin/wordle/StageTest.kt

+        assertThat(stage.state).isEqualTo(Stage.State.PROGRESS)
+        assertThat(stage.finished).isFalse()


다중 Assertions 을 사용할 경우, 최초 실패한 Assertions 문제만 콘솔에 표현될 수 있습니다.
AssertAll 을 사용해보는 것은 어떠실까요? 모든 Assertions 이 실행되고 실패 지점을 한번에 파악할 수 있는 장점이 있습니다 :D

관련 링크

Suggested change

assertThat(stage.state).isEqualTo(Stage.State.PROGRESS)

assertThat(stage.finished).isFalse()

assertAll(

{ assertThat(stage.state).isEqualTo(Stage.State.PROGRESS) },

{ assertThat(stage.finished).isFalse() }

)

kwj1270 · 2024-06-23T07:38:17Z

src/main/kotlin/wordle/domain/Step.kt

+        word.value.forEachIndexed { index, c ->
+            if (answer[index] == c) {
+                initResult[index] = Result.CORRECT
+            }
+        }


c 라는 네이밍 대신에 좀 더 명확한 네이밍이 있으면 좋을 것 같습니다 😄

오... 들켰군요! ㅋㅋㅋㅋ letter로 분리했습니다

kwj1270 · 2024-06-23T07:40:38Z

src/test/kotlin/wordle/WordTest.kt

+    @ValueSource(strings = ["test", "hi", "h", "tttttt"])
+    fun `5글자가 아니면 IllegalArgumentException 예외가 발생한다`(value: String) {
+        //when
+        val errorResponse = assertThrows<IllegalArgumentException> { Word.fromInput(value) }


assertThrows<IllegalArgumentException> 는 처음 보았는데 throwable 로 반환하는군요! 하나 배워갑니다 :D

- AnswerSelector 개념 추가 - Dictionary 확장하여 file 기반으로 명명

kwj1270

안녕하세요 우유님
피드백 반영 및 의도에 맞게 리팩토링하신 점 매우 좋았던 것 같습니다 👍🏻
사실 너무 깔끔하게 작성해주셔서 어느정도 만족은 하지만
약간의 오지랖으로 조금 더 코멘트 남겨보았습니다.
이번 3차 과제 고생 많으셨고, 개발 관련 논의할 내용 있으시면 편하게 말씀주세요.
수고하셨습니다 👏🏻 👏🏻 👏🏻

kwj1270 · 2024-07-01T14:37:10Z

src/main/kotlin/wordle/Game.kt

+import wordle.view.Output
+import java.time.LocalDate
+
+class Game(today: LocalDate) {


AnswerSelector 인터페이스가 생겼기에 today: LocalDate 의 위치도 더 적합한 곳으로 이동 가능해보일 것 같습니다 👍🏻

kwj1270 · 2024-07-01T14:38:34Z

src/main/kotlin/wordle/TodayAnswerSelector.kt

+import wordle.domain.AnswerSelector
+import java.time.LocalDate
+
+class TodayAnswerSelector(private val today: LocalDate): AnswerSelector {


이전 리뷰에 못 남겼는데 private val today: LocalDate 을 생성자 프로퍼티로 만든점 매우 좋은 것 같아요 👍🏻

kwj1270 · 2024-07-01T14:40:26Z

src/main/kotlin/wordle/domain/Stage.kt

+data class Stage(val answer: String, val steps: List<Step> = listOf()) {
+
+    enum class State {
+        PROGRESS, COMPLETE, FAIL
+    }
+


궁금한 점이 있습니다.!
사람마다 매우 다를 것 같은데 class 안에 inner class 를 선언한 이유가 있으실까요?
저도 어느 방법이 더 좋은지 잘 몰라서 질문 드려봅니다

kwj1270 · 2024-07-01T14:45:06Z

src/main/kotlin/wordle/domain/Word.kt

+    init {
+        require(value.matches(englishRegex)) { "영문만 입력해야합니다." }
+        require(value.length == 5) { "5글자여야 합니다." }
+    }


init 블록 사용 매우 좋습니다 👍🏻

kwj1270 · 2024-07-01T14:47:47Z

src/main/kotlin/wordle/domain/Word.kt

+    constructor(value: String, isLowercase: Boolean = false) :
+            this(if (isLowercase) value else value.lowercase())


저도 lowercase 로직이 들어오면서 구현이 복잡해지게 되더라고요 😭
원 문자열은 private val 형태로 소유하고 있되, 실제 사용할 때 lowercase 로 바꿔보는건 어떤가요?

cousim46 and others added 20 commits June 16, 2024 19:15

test : 단어 테스트

300705c

- 단어가 존재하지 않을 경우 - 단어가 존재 할 경우 - 현재날짜 기준으로 단어 추출

test : 게임 실행

0e29dbc

test : 스테이지 테스트

b034aab

- 스테이지 상태가 fail - 스테이지 상태가 progress일때 - 정답을 맞췄을때 - 정답을 맞추지 못했을때

test : 단어 테스트

dcad2b3

- 모두 영문으로 구성 - 5글자가 아닐 경우 - 영어를 대문자에서 소문자로 치환 - 단어가 존재하지 않을 경우

fix : jdk 버전 수정

8c77714

fix : indent 수정

f0c3b84

feat : 실행 파일 추가

cc8fa0b

feat : 사전 역할 생성

41e9aae

- 파일에서 특정 단어 추출 - 단어 존재여부 체크

feat : 게임 역할 생성

ef38d78

- 게임 시작

feat : 입력 역할 생성

2265953

- 단어 입력

feat : 출력 역할 생성

f4e7ba7

- 게임 시작 문구 - 정답 입력 요청 문구 - 결과 문구

feat : 스테이지 역할 생성

7702247

- 게임 진행 여부 - 게임 플레이

feat : 단계 역할 생성

7893ae7

- 단계별 답 체크

feat : 단어 역할 생성

37e4539

- 단어 5글자 여부 체크 - 단어 영문 여부 체크 - 존재 야부 체크 - 소문자로 치환

fix: Word 패키지 변경 및 require로 validation

9186d5e

fix: Step 패키지 변경 및 answer, word 포함

73296da

fix: Stage 패키지 변경 및 state 연산을 초기화시 진행

4c43baa

fix: Game, Application 수정 및 view/domain 레이어 추가

7980f69

fix: fail시 X/6 화면 노출

d24df44

kwj1270 reviewed Jun 23, 2024

View reviewed changes

woo-yu added 8 commits June 30, 2024 18:05

fix: dictionary 분리

1699aea

- AnswerSelector 개념 추가 - Dictionary 확장하여 file 기반으로 명명

fix: error 출력도 ouput으로 이동

397a5c7

fix: Word의 validation이 constructor를 활용

54bcaf4

fix: ! 사용 -> not()으로 변경

977b25e

fix: EOF

8a2da06

fix: Dictionary 잘못된 테스트 수정

2181344

test: test assertAll로 수정

925ea1f

fix: c -> letter로 수정

0595694

woo-yu requested a review from kwj1270 June 30, 2024 09:29

kwj1270 reviewed Jul 2, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Wordle] 호우팀(우유) 미션 제출합니다. #19

[Wordle] 호우팀(우유) 미션 제출합니다. #19

woo-yu commented Jun 16, 2024

kwj1270 Jun 23, 2024

kwj1270 left a comment

kwj1270 Jun 23, 2024 •

edited

Loading

woo-yu Jun 30, 2024

kwj1270 Jul 1, 2024 •

edited

Loading

kwj1270 Jun 23, 2024 •

edited

Loading

woo-yu Jun 30, 2024

kwj1270 Jul 1, 2024

kwj1270 Jun 23, 2024

woo-yu Jun 30, 2024

kwj1270 Jun 23, 2024

woo-yu Jun 30, 2024

kwj1270 Jul 1, 2024

kwj1270 Jun 23, 2024

kwj1270 Jun 23, 2024

woo-yu Jun 30, 2024

kwj1270 Jun 23, 2024

woo-yu Jun 30, 2024

kwj1270 Jul 1, 2024 •

edited

Loading

kwj1270 Jun 23, 2024

kwj1270 Jun 23, 2024

woo-yu Jun 30, 2024

kwj1270 Jun 23, 2024

kwj1270 left a comment

kwj1270 Jul 1, 2024

kwj1270 Jul 1, 2024

kwj1270 Jul 1, 2024

kwj1270 Jul 1, 2024

kwj1270 Jul 1, 2024

		assertThat(stage.state).isEqualTo(Stage.State.PROGRESS)
		assertThat(stage.finished).isFalse()

-        assertThat(stage.state).isEqualTo(Stage.State.PROGRESS)
-        assertThat(stage.finished).isFalse()
+assertAll(
+        { assertThat(stage.state).isEqualTo(Stage.State.PROGRESS) },
+        { assertThat(stage.finished).isFalse() }
+)

		constructor(value: String, isLowercase: Boolean = false) :
		this(if (isLowercase) value else value.lowercase())

[Wordle] 호우팀(우유) 미션 제출합니다. #19

Are you sure you want to change the base?

[Wordle] 호우팀(우유) 미션 제출합니다. #19

Conversation

woo-yu commented Jun 16, 2024

Choose a reason for hiding this comment

kwj1270 left a comment

Choose a reason for hiding this comment

kwj1270 Jun 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kwj1270 Jul 1, 2024 • edited Loading

Choose a reason for hiding this comment

kwj1270 Jun 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kwj1270 Jul 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kwj1270 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kwj1270 Jun 23, 2024 •

edited

Loading

kwj1270 Jul 1, 2024 •

edited

Loading

kwj1270 Jun 23, 2024 •

edited

Loading

kwj1270 Jul 1, 2024 •

edited

Loading