-
Notifications
You must be signed in to change notification settings - Fork 35
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Wordle] 파랑(이하은) 미션 제출합니다. #13
base: main
Are you sure you want to change the base?
Conversation
README.md
Outdated
- [ ] 시작 문구를 띄워준다. | ||
- [ ] 답안을 입력받는다. | ||
- [x] 답안은 5글자여야 한다. | ||
- 답안은 `words.txt` 안에 존재하는 단어여야 한다. | ||
- [ ] 정답과 답안을 비교하여 결과를 타일로 표현한다. | ||
- 맞는 글자는 초록색으로 표현한다. | ||
- 위치가 틀리면 노란색으로 표현한다. | ||
- 정답에 존재하지 않는 글자면 회색으로 표현한다. | ||
- 두 개의 동일한 문자를 입력하고 그중 하나가 회색으로 표시되면 해당 문자 중 하나만 최종 단어에 나타난다. | ||
- [x] 정답은 매일 바뀐다. | ||
- [x] `words.txt` 안의 ((현재 날짜 - 2021년 6월 19일) % 배열의 크기) 번째의 단어이다. | ||
- [ ] 정답을 맞췄을 경우 타일 표현 전에 몇 번째 만에 맞췄는지 출력한다. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
기능 요구 사항을 잘 정리해주셨는데 아직 [x] 업데이트가 아직 안된 것 같아요!
var count: Int = 0 | ||
private set | ||
|
||
var results: MutableList<List<Tile>> = ArrayList() | ||
private set |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
private set
👍 👍 좋은방법같아요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
그때 제로가 질문해서 배웠습니다👍
private set | ||
|
||
fun isGameOver(answer: Word): Boolean { | ||
return count == 6 || words.isCorrect(answer) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OutputView에서 WORDLE을 6번 만에 맞춰 보세요.
의 6
과 도메인 Game의 6
의 의존 값을 각각 가지고 있는 것 같아요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
그렇네요 Game에서 public 상수로 선언해서 OutputView에서 가져다 쓰는 것으로 변경했습니다~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
깰끔하네여 ㅎ
init { | ||
require(value.length == 5) { "단어의 길이는 5글자여야 합니다." } | ||
require(Regex("[a-zA-Z]*").matches(value)) { "단어에 영어가 아닌 글자나 공백이 포함될 수 없습니다." } | ||
} | ||
|
||
fun isSameChar(other: Word, index: Int): Boolean { | ||
return this.value[index] == other.value[index] | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
깔끔 하네요ㅎㅎ 💯
val result: ArrayList<Tile> = ArrayList() | ||
repeat(5) { result.add(Tile.GRAY) } | ||
|
||
repeat(5) { i -> result[i] = findTileBySameCheck(word, i) } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
repeat(5) { i -> result[i] = findTileBySameCheck(word, i) } | |
repeat(5) { result[it] = findTileBySameCheck(word, it) } |
이렇게도 될거같아요!
class Words(private val values: List<Word>) { | ||
|
||
private val answer: Word = findAnswer() | ||
private var answerMap: MutableMap<Char, Int> = HashMap() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HashMap
을 사용한 이유가 궁금해요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
문자를 비교해서 같은 경우 문자를 소진하기 때문에 문자의 개수를 저장할 필요가 있었습니다. 그렇기 때문에 문자 - 문자의 개수를 key, value로 저장하기 위해 HashMap을 사용했습니다! 이 부분이 궁금했던 거 맞나요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이해했습니다!! 역시 S....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코틀린은 읽기 전용 컬렉션과 변경 가능한 컬렉션을 구별해 제공합니다.
private var answerMap: MutableMap<Char, Int> = HashMap() | |
private var answerMap: MutableMap<Char, Int> = mutableMapOf() |
object WordsReader { | ||
|
||
fun getWords(): List<Word> { | ||
val path = "src/main/resources/words.txt" | ||
val reader = FileReader(path) | ||
return reader.readLines().map { Word(it) } | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WordsReader
좋네요!! 근데 해당 클래스가 Util
패키지에 속하는 것이 맞는지 궁금해요. 저도 잘 몰라서 질문드립니다 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
txt 파일에서 문자열을 읽어와 Word list로 변환해주는 역할만 하고 있어서 util 클래스라고 생각했습니다!
import io.kotest.matchers.shouldBe | ||
import org.junit.jupiter.api.Test | ||
|
||
class WordTest { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코테스트 배워갑니다. 👍 👍 👍 👍 👍 👍
파랑 안녕하세요 ^-^ 파랑이 구현한 코드를 보고 한번 제 의견 달아봤습니다. 코드를 보면서 저도 다시 한번 배울 수 있었습니다 👍 요즘 바쁜데 힘 같이 내봐요 파랑 화이티리링~ 🟦 🔢 💙 🈂️ 🔤 🧊 |
fun match(answer: Word) { | ||
require(words.contains(answer)) { "등록된 단어가 아닙니다." } | ||
count++ | ||
results.add(words.check(answer)) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
results.add(words.check(answer))
에서 오류가 발생하더라도 count
가 증가할 것 같아요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
그렇네요 수정했습니다!
fun main() { | ||
val game = Game(WordsReader.getWords()) | ||
OutputView.printStartMessage() | ||
doGame(game) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doGame
보다 playGame
은 어떠세요?ㅎㅎ 파랑은 어떻게 생각하시나요!?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
play game이 더 자연스러운 것 같아서 변경했습니다👍
코틀린 장인을 꿈꾸는 파랑... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
잘 했어요. 👍 조금 더 수정이 필요한 것 같아요.
가능한 읽기 전용 컬렉션을 사용하면 어떨까요?
src/main/kotlin/study/skill/Skill.kt
Outdated
@@ -0,0 +1,3 @@ | |||
package study.skill | |||
|
|||
interface Skill |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Skill
, HardSkill
, SoftSkill
을 하나의 파일로 모으고 봉인된 클래스를 사용해 볼까요?
https://kotlinlang.org/docs/sealed-classes.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sealed class는 이번에 처음 알게 되었네요! 말씀해주신대로 하나의 파일로 모아 Skill을 봉인된 클래스로 만들었습니다!
OutputView.printCount(game.count) | ||
OutputView.printResults(game.results) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
View에 대한 호출을 최소화해 보면 어떨까요?
return | ||
} | ||
OutputView.printResults(game.results) | ||
playGame(game) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
재귀 함수보다 조금 더 직관적으로 만들 수 있을까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
while문을 사용하는 것으로 로직을 수정했습니다.
var count: Int = 0 | ||
private set | ||
|
||
var results: MutableList<List<Tile>> = ArrayList() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
뒷받침하는 프로퍼티(backing property)를 사용해 보세요.
|
||
init { | ||
require(value.length == 5) { "단어의 길이는 5글자여야 합니다." } | ||
require(Regex("[a-zA-Z]*").matches(value)) { "단어에 영어가 아닌 글자나 공백이 포함될 수 없습니다." } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
String
을 패턴 매칭을 위해 Regex
으로 바꾸는 것은 꽤나 비싼 행위입니다. 미리 Regex
으로 만들어 사용하는 것은 어떨까요? 아래의 글을 참고하세요.
https://stackoverflow.com/questions/2469244/whats-the-difference-between-string-matches-and-matcher-matches/2469275
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regex
는 실행될 때마다 리컴파일되는 군요.. pattern을 상수로 만들어두고 pattern.matcher
를 사용하는 것으로 변경했습니다!
class Words(private val values: List<Word>) { | ||
|
||
private val answer: Word = findAnswer() | ||
private var answerMap: MutableMap<Char, Int> = HashMap() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코틀린은 읽기 전용 컬렉션과 변경 가능한 컬렉션을 구별해 제공합니다.
private var answerMap: MutableMap<Char, Int> = HashMap() | |
private var answerMap: MutableMap<Char, Int> = mutableMapOf() |
private fun initAnswerMap(): MutableMap<Char, Int> { | ||
val answerMap: MutableMap<Char, Int> = HashMap() | ||
answer.value.forEach { | ||
answerMap[it] = answerMap.getOrDefault(it, 0) + 1 | ||
} | ||
return answerMap | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아래의 코드로 개선해 보면 어떨까요?
private fun initAnswerMap(): MutableMap<Char, Int> { | |
val answerMap: MutableMap<Char, Int> = HashMap() | |
answer.value.forEach { | |
answerMap[it] = answerMap.getOrDefault(it, 0) + 1 | |
} | |
return answerMap | |
} | |
private fun initAnswerMap(): Map<Char, Int> { | |
return answer.value | |
.groupBy { it } | |
.mapValues { (_, values) -> values.count() } | |
} |
object OutputView { | ||
|
||
fun printStartMessage() = println( | ||
"WORDLE을 ${Game.MAX_GAME_COUNT}번 만에 맞춰 보세요.\n" + "시도의 결과는 타일의 색 변화로 나타납니다." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아래의 코드로 개선해 보면 어떨까요?
"WORDLE을 ${Game.MAX_GAME_COUNT}번 만에 맞춰 보세요.\n" + "시도의 결과는 타일의 색 변화로 나타납니다." | |
"WORDLE을 ${Game.MAX_GAME_COUNT}번 만에 맞춰 보세요.\n시도의 결과는 타일의 색 변화로 나타납니다." |
@Test | ||
fun `게임 종료여부를 확인한다`() { | ||
val game = Game(listOf(Word("apple"), Word("hello"), Word("spicy"))) | ||
repeat(6) { game.match(Word("apple")) } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
매번 이렇게 match()
를 사용해야 할까요? 어떻게 하면 더 손쉬운 테스트를 할 수 있을까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Game을 생성할 때 maxGameCount를 받아 match를 사용하지 않고도 테스트할 수 있도록 수정했습니다!
제이슨 안녕하세요! 파랑입니다. |
마지막 PR 제출합니다. 이번 워들 미션 덕분에 코틀린에 대해 많이 알아갈 수 있었습니다. 아래 내용들을 중심으로 리팩토링을 진행했습니다. 감사합니다!
|
안녕하세요 파랑입니다.
워들 미션 제출합니다. 감사합니다!