[Wordle] 페퍼(최수연) 미션 제출합니다. #10

SuyeonChoi · 2022-05-13T10:20:43Z

안녕하세요 리뷰어 케이님!
저의 wordle 미션 잘 부탁드리겠습니다🤗

kth990303

안녕하세요 페퍼! 이번 코틀린 워들 리뷰를 맡게 된 케이입니다 😄
매우 잘해주셨네요 ㅎㅎ 몇 가지 코멘트 남겨드렸어요!
저도 덕분에 공부가 많이 됐구요 :) 감사합니다.
올해 지원 플랫폼 근로 같이 화이팅해봐요!

kth990303 · 2022-05-14T07:03:16Z

src/test/kotlin/wordle/domain/AnswerTest.kt

+import org.assertj.core.api.Assertions.assertThat
+import org.junit.jupiter.api.Test
+import org.junit.jupiter.api.assertThrows
+import wordle.domain.Mark.*


./gradlew ktlintCheck가 실패하네요!

import문에 와일드카드를 사용하지 않는 방법이 좋아보입니다 😄

반영하였습니다!

kth990303 · 2022-05-14T07:04:40Z

src/test/kotlin/wordle/domain/GameTest.kt

+
+        assertAll(
+            {assertThat(game.findTryCount()).isEqualTo(3)},
+            {assertThat(game.isPlaying).isTrue()}


ktlintCheck가 실패하네요 😢
중괄호 사이에 공백이 있는 게 좋아보입니다.

ktlintCheck이 통과되도록 수정하였습니다👍
마지막에 확인하는 습관을 잊지 않아야겠어요!

kth990303 · 2022-05-14T07:10:13Z

src/main/kotlin/wordle/controller/WordleController.kt

+            val answer = Answer(printInputMessage())
+            game.playRound(answer)
+            printResults(game.results, game.isPlaying, game.findTryCount())
+        }


올바르지 않은 단어를 입력했을 때 재입력받게 하면 유저들이 더 편하게 게임을 이용할 수 있어보이네요!

게임이 종료되는 상황을 피할 수 있겠군요~! try-catch문을 활용해서 반영해보았습니다.

kth990303 · 2022-05-14T07:14:16Z

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

+    }
+
+    private fun matchExact(word: String, result: MutableList<Mark>, wordTable: HashMap<Char, Int>) {
+        for (i in 0 until WORD_SIZE) {


이렇게 map으로 처리하는 방법도 있겠네요! 꼭 이 방법이 더 좋다는건 아니고, 또 다른 방법도 존재한다는 내용의 코멘트입니다.

Suggested change

for (i in 0 until WORD_SIZE) {

(0 until WORD_SIZE).map { markExact(it, word, result, wordTable) }

kth990303 · 2022-05-14T07:19:38Z

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

+        }
+
+        fun pick(date: LocalDate): String {
+            val index = ChronoUnit.DAYS.between(BASIC_DATE, date) % VALUE.size


ChronoUnit.DAYS 사용 좋네요! 배워갑니다 👍
추가로, 페퍼의 코드 덕분에 ChronoUnit의 between과 Period의 between 차이를 공부해볼 수 있었어요 :)

케이님의 링크 덕분에 저도 다시한번 복습했네요😄😄

kth990303 · 2022-05-14T07:47:34Z

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

+
+class Words {
+
+    companion object {


companion object을 활용하여 캐싱한건가요? 👍

넵 불필요한 데이터를 매번 생성하지 않게끔 활용해보았습니다〰️

kth990303 · 2022-05-14T07:49:09Z

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

+        when (it) {
+            Mark.NONE -> stringBuilder.append("⬜")
+            Mark.EXIST -> stringBuilder.append("\uD83D\uDFE8")
+            Mark.EXACT -> stringBuilder.append("\uD83D\uDFE9")


NONE은 도형을, EXACT, EXIST는 유니코드를 사용한 이유는 무엇인가요?
+) StringBuilder를 사용한 이유는 무엇인가요?

예시 출력을 그대로 붙여넣었더니 통일성 있지 않게 삽입되었네요🥲 통일성을 위해 모두 도형으로 변경하였습니다⬜🟨🟩

매번 String을 생성하여 출력하는 것보단, 가변적인 StringBuilder()를 사용하여 필요한 문자열을 모두 더한 뒤, 한번에 출력하는 것이 효율적이라고 판단하여 사용해보았습니다!

kth990303 · 2022-05-14T07:50:26Z

src/test/kotlin/wordle/domain/AnswerTest.kt

+
+    @Test
+    fun 글자길이가_5가_아닌_경우_예외발생() {
+        assertThrows<IllegalArgumentException> { Answer("abcdef") }


forAll을 이용하여 4글자인 경우도 테스트해보면 좋을 것 같아요!

경계값을 모두 테스트하는 것이 좋겠군요! 반영하였습니다〰️

kth990303 · 2022-05-14T07:53:49Z

src/test/kotlin/wordle/domain/AnswerTest.kt

+
+        assertThat(answer.compareToWord("sheen"))
+            .isEqualTo(listOf(EXIST, EXIST, NONE, NONE, NONE))
+    }


아래의 경우도 테스트해보면 어떨까요?

EXACT만 존재하는 경우

NONE만 존재하는 경우

EXIST만 존재하는 경우 (ex. parse, spear)

중복되는 문자가 존재하면서, 입력한 단어에 그 문자가 더 많은 경우

그 외에 다양한 경우들이 존재할 것 같아요.

테스트가 많이 부족했던거 같아요! 워들 세부 규칙의 예시를 참고하여 추가적인 테스트를 만들어보았습니다🙂

kth990303 · 2022-05-14T07:54:41Z

src/test/kotlin/wordle/domain/AnswerTest.kt

+
+    @Test
+    fun 글자길이가_5가_아닌_경우_예외발생() {
+        assertThrows<IllegalArgumentException> { Answer("abcdef") }


assertThrow문 사용하셨군요! 👍 저는 kotest의 shouldThrow를 사용했는데 둘 다 좋네요 ㅎㅎ

이전에는 아직 kotest가 익숙하지 않았어요.
이번에 차리와 케이 코드를 보다가 kotest에 관심이 생겨서 변경해보았습니다! 확인해주시면 감사하겠습니다🙏

- Input과 Output 뷰를 구분하기 위해, 입력받는 메서드명 printInputMessage -> requestInput 변경

- refactor(Answer): 불필요한 주석 제거

SuyeonChoi

안녕하세요 케이! 꼼꼼한 리뷰 감사합니다~!
케이의 금쪽같은 리뷰도 반영하고 dsl 실습도 진행해보았습니다😊

SuyeonChoi · 2022-05-22T12:12:01Z

src/main/kotlin/wordle/controller/WordleController.kt

+            val answer = Answer(printInputMessage())
+            game.playRound(answer)
+            printResults(game.results, game.isPlaying, game.findTryCount())
+        }


게임이 종료되는 상황을 피할 수 있겠군요~! try-catch문을 활용해서 반영해보았습니다.

SuyeonChoi · 2022-05-22T12:28:13Z

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

+        when (it) {
+            Mark.NONE -> stringBuilder.append("⬜")
+            Mark.EXIST -> stringBuilder.append("\uD83D\uDFE8")
+            Mark.EXACT -> stringBuilder.append("\uD83D\uDFE9")


예시 출력을 그대로 붙여넣었더니 통일성 있지 않게 삽입되었네요🥲 통일성을 위해 모두 도형으로 변경하였습니다⬜🟨🟩

매번 String을 생성하여 출력하는 것보단, 가변적인 StringBuilder()를 사용하여 필요한 문자열을 모두 더한 뒤, 한번에 출력하는 것이 효율적이라고 판단하여 사용해보았습니다!

SuyeonChoi · 2022-05-22T12:31:05Z

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

+class Words {
+
+    companion object {
+        private val VALUE: List<String> = Files.readAllLines(Paths.get("src/main/resources/words.txt"))


이런 방법이!!! 감사합니다 리뷰어님🙇‍♀️

SuyeonChoi · 2022-05-22T12:33:22Z

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

+
+class Words {
+
+    companion object {


넵 불필요한 데이터를 매번 생성하지 않게끔 활용해보았습니다〰️

SuyeonChoi · 2022-05-22T12:34:17Z

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

+        }
+
+        fun pick(date: LocalDate): String {
+            val index = ChronoUnit.DAYS.between(BASIC_DATE, date) % VALUE.size


케이님의 링크 덕분에 저도 다시한번 복습했네요😄😄

SuyeonChoi · 2022-05-22T13:30:49Z

src/test/kotlin/wordle/domain/AnswerTest.kt

+
+    @Test
+    fun 글자길이가_5가_아닌_경우_예외발생() {
+        assertThrows<IllegalArgumentException> { Answer("abcdef") }


이전에는 아직 kotest가 익숙하지 않았어요.
이번에 차리와 케이 코드를 보다가 kotest에 관심이 생겨서 변경해보았습니다! 확인해주시면 감사하겠습니다🙏

SuyeonChoi · 2022-05-22T13:40:40Z

src/test/kotlin/wordle/domain/AnswerTest.kt

+
+    @Test
+    fun 글자길이가_5가_아닌_경우_예외발생() {
+        assertThrows<IllegalArgumentException> { Answer("abcdef") }


경계값을 모두 테스트하는 것이 좋겠군요! 반영하였습니다〰️

SuyeonChoi · 2022-05-22T17:38:23Z

src/test/kotlin/wordle/domain/AnswerTest.kt

+
+        assertThat(answer.compareToWord("sheen"))
+            .isEqualTo(listOf(EXIST, EXIST, NONE, NONE, NONE))
+    }


테스트가 많이 부족했던거 같아요! 워들 세부 규칙의 예시를 참고하여 추가적인 테스트를 만들어보았습니다🙂

SuyeonChoi · 2022-05-22T17:43:42Z

src/test/kotlin/wordle/domain/GameTest.kt

+
+        assertAll(
+            {assertThat(game.findTryCount()).isEqualTo(3)},
+            {assertThat(game.isPlaying).isTrue()}


ktlintCheck이 통과되도록 수정하였습니다👍
마지막에 확인하는 습관을 잊지 않아야겠어요!

SuyeonChoi · 2022-05-22T17:43:57Z

src/test/kotlin/wordle/domain/AnswerTest.kt

+import org.assertj.core.api.Assertions.assertThat
+import org.junit.jupiter.api.Test
+import org.junit.jupiter.api.assertThrows
+import wordle.domain.Mark.*


반영하였습니다!

kth990303

안녕하세요 후추! 피드백 반영 확인했고, 잘 반영해주셨어요.
후추가 리뷰를 빠르게 먼저 보내달라해서 이정도 보내둘게요 ㅎㅎ
주말에 이어서 피드백해드릴 예정이에요! 그 전까지 리팩터링하고 싶다면 리팩터링해서 또 보내주세요~
수고많으셨습니다

kth990303 · 2022-05-26T09:03:31Z

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

+class Words {
+
+    companion object {


현재 Words 클래스에 companion object밖에 없는데, Words 클래스 자체를 object 클래스로 만드는 건 어떤가요?

객체를 생성할 일이 없으니 object클래스를 사용하는 방식이 더 낫겠네요! 반영하였습니다~!

kth990303 · 2022-05-26T09:04:05Z

src/test/kotlin/study/DslTest.kt

+data class Skills(val hard: MutableList<String>, val soft: MutableList<String>) {
+    fun hard(value: String) {
+        hard.add(value)
+    }
+
+    fun soft(value: String) {
+        soft.add(value)
+    }
+}


저는 SkillBuilder를 따로 만들어줬는데, 이 방법도 좋네요!
그 외 부분은 거의 동일하네요 ㅎㅎ

kth990303

안녕하세요 후추 😄
너무 잘해주셔서 사실 아무것도 피드백드리지 않으려다가, 딱 한가지 개선이 필요할 것 같아서 (그리고 한 가지 공유해드리고 싶은 링크도 있어서) 코멘트 남겨드려요~
사실 DSL 테스트 쪽에서 많이 피드백드리고 싶었는데, 저랑 코드가 거의 비슷하기도 하고, 제가 아직 충분한 공부가 안돼서인지 피드백 드릴 부분이 없더라구요 ㅎㅎ
ktlintCheck만 통과되면 너무 좋을 것 같습니다~

kth990303 · 2022-05-29T17:36:19Z

src/test/kotlin/wordle/domain/AnswerTest.kt

+import wordle.domain.Mark.NONE
+import wordle.domain.Mark.EXIST
+import wordle.domain.Mark.EXACT


이건 제가 생각하기에도 ktlintCheck가 좀 너무한 것 같은데, 사전순 정렬로 된 import가 아니라서 ktlintCheck가 실패하네요...

그동안의 빌드 작업을 clean하지 않았더니 로컬에선 계속 BUILD SUCCESS가 되었었네요😂😂
./gradlew clean ktlintCheck로 오류 확인하고 해결하였습니다 :D

kth990303 · 2022-05-29T17:54:48Z

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

+        for (char in word) {
+            wordTable[char] = wordTable.getOrDefault(char, 0) + 1
+        }


이 작업은 map으로도 똑같이 가능하더라고요! 그래서 map이랑 foreach랑 차이점이 뭘까 생각이 들어서 한번 검색해봤는데, 이러한 차이가 있었습니다. (Is there a difference between foreach and map?)

만약 HashMap이 아닌 List나 Set이었으면 map을 이용하는 것이 의미가 있었을 것 같은데, HashMap이어서 forEach가 더 좋아보이네요. 결론은 foreach 잘썼다 👍

유용한 링크도 첨부해주셔서 저도 덕분에 함께 공부했습니다😊😊 감사합니다!

kth990303 · 2022-05-29T17:55:46Z

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

+        if (isOver(result)) {
+            isPlaying = false
+        }


이건 정말 쓸모없는 팁이긴 하지만, 아래처럼 쓸 수도 있습니다 ㅎㅎ

isPlaying = !isOver(result)

오! 코드 길이를 줄일 수 있는 유용한 팁인걸요?! 바로 반영하였습니다!
역시 리뷰어 kth!👏👏

SuyeonChoi · 2022-05-31T14:09:40Z

개인적으로 아쉬운 부분이 많지만 워들 미션을 너무 오래하고 있어서 일단 피드백만 반영해보았습니다.
내일 쉬는 날인데 코틀린 공부도 화이팅해봐야겠어요🙃🙃
아마 마지막 리뷰 요청이 될 거 같네요!
제 코드 리뷰 하시느라 그동안 수고 많으셨고, 케이 리뷰어를 만나서 영광이였습니다! 👏😆👏

kth990303 · 2022-05-31T14:22:16Z

수고 많으셨습니다! 저도 정말 많이 배웠어요.
앞으로 코틀린 스터디랑 근로 같이 하면서 둘 다 코틀린 왕이 되봅시다~ㅎㅎ

jojogreen91 added 11 commits May 11, 2022 20:38

docs: README.md 기능목록 추가

a07ca44

feat: Answer 글자길이를 검증하는 기능 구현

13a13cf

test: AnswerTest 테스트작성

c1d7d33

feat: Answer 가 words.txt 에 존재하는 단어인지 검증하는 기능

778f9c1

feat: 오늘의 정답을 선택하는 기능

0782fbf

feat: 답안과 정답을 비교하는 기능 구현

58cbf75

feat: 입출력 기능 구현

551f64f

feat: Wordle 게임을 실행하는 기능 구현

f0c74c9

feat: 어플리케이션을 실행하는 기능 구현

d9a554d

test: Game 테스트 구현

8f1f5f0

test: 정답과 비교하는 테스트 케이스 추가 구현

5422144

kth990303 reviewed May 14, 2022

View reviewed changes

SuyeonChoi added 11 commits May 22, 2022 21:10

feat: 유저의 입력이 잘못된 경우, 재입력을 받도록 변경

92ee394

- Input과 Output 뷰를 구분하기 위해, 입력받는 메서드명 printInputMessage -> requestInput 변경

refactor: 유니코드를 도형 아이콘으로 변경

7fbf22a

refactor: 불필요한 코드 제거 및 개선

4dc585c

test: 백틱을 사용하여 테스트 함수명 변경

91d964c

test: kotest를 적용하여 테스트 코드를 변경

98a1af3

test(AnswerTest): 글자길이가 5가 아닌 경우의 경계값 테스트 추가

aaaad52

test(GameTest): 경계를 테스트하도록 변경

066b045

test(AnswerTest): 테스트 케이스 추가

0350518

- refactor(Answer): 불필요한 주석 제거

refactor(AnswerTest): wildcard import문 제거

084a409

test: DslTest 수업 및 실습 코드 추가

3c355c2

style: 코드 리포맷팅

9dae55e

SuyeonChoi commented May 22, 2022

View reviewed changes

refactor: 단어를 맞출 수 있는 횟수를 Game에 선언하여 사용하도록 변경

9e1193a

kth990303 reviewed May 26, 2022

View reviewed changes

refactor(Words): object 클래스로 변경

3e3219e

kth990303 reviewed May 29, 2022

View reviewed changes

SuyeonChoi added 2 commits May 31, 2022 22:59

refactor(Game): 불필요한 코드 길이 개선

5e3965e

style(AnswerTest): import 순서가 컨벤션을 지키도록 변경

4f9251e

	for (i in 0 until WORD_SIZE) {
	(0 until WORD_SIZE).map { markExact(it, word, result, wordTable) }

[Wordle] 페퍼(최수연) 미션 제출합니다. #10

Are you sure you want to change the base?

[Wordle] 페퍼(최수연) 미션 제출합니다. #10

Conversation

SuyeonChoi commented May 13, 2022

kth990303 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

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

Choose a reason for hiding this comment

SuyeonChoi 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

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

kth990303 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

kth990303 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

Choose a reason for hiding this comment

SuyeonChoi commented May 31, 2022 • edited Loading

kth990303 commented May 31, 2022

SuyeonChoi commented May 31, 2022 •

edited

Loading