Skip to content
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] 돌쓰팀(보라돌) 미션 제출합니다. #21

Open
wants to merge 40 commits into
base: main
Choose a base branch
from

Conversation

boradol
Copy link

@boradol boradol commented Jun 17, 2024

페어프로그래밍으로 하는 워들 게임 👻

유스🚎님과 함께하는 보라돌의 Kotlin 여행기였습니다.

페어프로그래밍 과정

  • 언어 : 코틀린

  • 페어프로그래밍 방식

    1. 온라인 - 게더+Code With Me : 이것 참 신세계네요! 근데 Host는 편한데 Guest는 단축키나, 자동완성 같은 것이 지원되지 않아요.
    2. 오프라인 - 테크살롱 : 테크살롱에는 페어룸이 있어 좋았습니다. 큰 모니터로 보며 하니 능률이 살고 좋았습니다.
      무엇보다도 제이슨🤓님이 계시는 곳!!!! 야호!!!!
    3. 오프라인 - 공유오피스 : 공용모니터도 있고, 회의실도 있고 깔끔하고 좋았습니다. 이 때, 다른 팀들과 함께하여 더 즐거웠습니다.

페어프로그래밍 후기

  1. 첫번째 미팅 때 게더에서 진행하였고, 모각코 전에 미리 요구사항과 워들 게임을 구현하고 git으로 공유 보기로 하였습니다.

  2. 두번째 미팅은 테크살롱에서 진행되었고, 직접 대면으로 하다보니 소통하는데 좋았습니다.

    • 처음 10분정도 드라이버와 네이게이터를 하다가 텀이 너무 짧다는 것을 느꼈습니다. 그래서 25~30분정도가 적당하다고 느꼈습니다.
    • 키보드를 집에서 들고 갔는데 페어프로그래밍 할 때, 효율적이었던 것 같습니다.
    • 미리 구현하고 만나서 TDD를 해볼 수 있을 줄 알았는데 점점 진행될 수록 구현에 집중하게 되었습니다. 생각보다 어려운 TDD입니다.
    • 요구사항에 대해 정리하면서 객체이름이나 기능명세를 구체화하는 시간이었습니다. 미리 둘다 구현해서 그런지 금방 완료하였습니다.
    • 이 날은 어떤 객체에게 책임을 줄지에 대해 고민을 많이 했던 시간이었습니다. 유스님 덕분에 역할나누는 것이 크게 힘들지 않았습니다.
    • 근데 이름짓는게 너무 힘들었습니다. 클래스명, 함수명 언제쯤 잘 지을 수 있을까?
    • 어떻게 하면 테스트 코드를 짜기 쉬운 구조가 될까에 대해서 유스님께 많이 배웠습니다. 갓갓갓 👍 그저 감탄 빛 유스님✨
    • 오래간만에 코틀린을 하는거라 제가 많이 뚝딱였는데 코틀린 천재 유스님의 지휘아래 잘 진행되었습니다.
    • 의사소통할 때 제가 무근본 코더라서 우유부단한 면이 있었는데 유스님이 설명도 잘해주시고 실무 관점에서 많은 조언을 들었습니다.
    • 페어프로그래밍 하니 친밀도가 훅 올라갔습니다. 좋은 분과 페어라니 하나님께 감사인사🙏🏻를 드렸습니다.
  3. 세, 네번째 미팅은 게더와 Code With Me를 사용하였습니다.

    • 유스님이 최근 야근이 많으신데도 시간을 내어주시고, 시작 할 때부터 파이팅이 넘치셔서 재밌게 페어 프로그래밍 하였습니다.
    • 워들 게임 로직을 완성하는 시기였습니다.
    • 미리 구현해본 코드에 로직상 미흡한 부분이 있었는데 유스님 주도하에 차근히 디버깅 하면서 문제해결하는 과정이었습니다.
    • 온라인으로 듣는 코틀린 과외의 현장이었습니다. 갓갓갓 👍👍👍 유스버스 과외 슨상님 감사합니다.🙇🏻‍♀️
    • 유스님쪽에서 단축키랑 자동완성이 힘들어 제가 드라이버 역할을 주로 하게 되었는데요. 생각보다 피로도가 높았습니다.
    • 그래도 차분한 유스님이 덕분에 제가 헤매고 있으면, 잠시만 제가 키보드 쓸게요!🙋🏻 하시면서 천재 코더⌨️의 면모를 보여주셨습니다.
    • 최근 바쁜 업무로 시달리시는 중인데도 전혀 타격없이 평일 밤에도 열일 해주신 유스님 감사합니다!
  4. 다섯번째 미팅은 공유오피스에서 진행되었습니다.

    • 다른 페어 두팀과 모여서 진행하였습니다. 서로의 프로그래밍 진행상황과 각자 리뷰를 해주면서 배운점이 많았습니다.
    • 다른 팀의 코드를 보고나니 우리 팀 코드의 개선점이나 아이디어가 떠올랐습니다. 리팩토링 요소 5개 이상 찾기 시간을 가졌습니다.
    • 객체에게 물어보기, 역할주기에 대해 중점적으로 하였고, 코틀린스럽게코드 조직하는 것이 주된 내용이었습니다.
    • 계속해서 배웠지만 회수가 거듭될 수록 천재 유스버스님의 코드를 보면서 도파민이 터지기🤯 시작했습니다. 굉장히 흥미로웠습니다.
    • 다른 팀의 코드를 보고 어디까지가 추상화 할 것인지에 대해 이야기 나누는 시간을 가졌습니다.
  5. 여섯번째 미팅은 테크살롱에서 진행하였습니다.

    • 예정에 없던 미팅이었지만 역시 페어는 오프라인이 좋은 것 같습니다.👍
    • 무한 코틀린 스럽게 리팩토링 하기, 테스트 코드 작성Ktlint 적용을 주로 하였습니다.
      (라고 적고 일타 유스님의 과외현장이라 읽습니다.)
    • 유스님은 테스트코드를 잘 작성하시더라고요! 겸손의 말씀을 하지만 덕분에 거북이 같은 제 속도로 불가능했던 일들이 가능해졌습니다.
    • 오늘 생각해보니 어느새 드라이버와 네비게이터의 경계가 허물어 졌던 것 같아요.
      근데 제생각으론 순탄하게 진행되었습니다. 서로 의견도 적극적으로 내고, 서로 코드스타일이 적응이 되어 한 팀이 된 것 같았습니다.
    • 기존 코드를 커밋단위를 잘 분리하여 PR올리는 것까지 목표였으나 여기까지 진행하지 못하였습니다.
    • 처음에는 페어프로그래밍의 기간 여유가 많다고 생각하였는데 시간이 부족하다 느꼈습니다. 하지만 혼자 하는 것 보다 코드의 퀄이 좋은 것 같습니다.
    • Kotlin 명강사 제이슨님께 코틀린 관련 질문도 하고 TDD의 어려움을 토로했는데 다 지우고 시작해보는 조언을 얻었습니다.
    • 커밋을 관리하지 않아 숙제로 남기고 마무리를 하게 되었습니다.
      커밋 이력을 안한 것이 후회되다가 각자 커밋을 쪼개보면서 또 공부가 될 것 같다고 생각합니다.

멋진 짱 개발자 유스님께

유스님 저와 함께 미션 하시느라 고생 많으셨어요 👍
페어프로그래밍 후기를 작성하려고 했는데 작성하다보니 유스 용비어천가🎶가 되어버렸네요. 근데 사실이니까요!!
함께 하면서 많이 친해지기도 하였지만, 유스님께서 저를 많이 배려해주신 것 같아 편하고 즐겁게 페어프로그래밍 하였어요!
또, 제가 명확히 아는 것이 많이 없어 의사소통을 잘할 수 있을까? 걱정되었는데 유스님이 척척 잘 해주셔서 저는 SRT를 타버리고 말았습니다.
코틀린 그리고 좋은 동료 대해 많이 배웠습니다. 진짜 무한 감사합니다. 🙇🏻‍♀️

페어프로그래밍 후 코드 변경 점

커밋 단위를 나누다 보니 또 리팩토링 할 것이 보였습니다.

  1. 요구사항정리 부분을 크게 한판 정리하였습니다. (README.md)
    프로그래밍 요구사항, 용어정리, 기능 목록을 페어프로그래밍 진행 후에 우리 스타일에 맞게 변경하였습니다.
    그리고 어떻게하면 다른사람이 보기에 쉬운 말로 표현할 수 있을까에 대한 고민도 했고,
    요구사항을 구현하고 나면 체크박스에 체크를 하고 기능 명세도 계속 수정하며 커밋하였습니다.

  2. 파일에서 워들게임에 필요한 단어목록을 불러오는 DictionaryFileLoader
    외부에 참조되어 있는 영역이라 생각하여 infra영역으로 패키지명을 변경하였습니다.
    그러면서 도메인들이 infra영역에 있는 것을 바로 참조하지 않게 Dictionary라는 도메인을 추가로 만들게 되었습니다.
    그리고 DictionaryFileLoader에서 단어 목록들을 List에서 contains를 확인하지 않고 Set을 사용하여 성능적으로 개선하였습니다.

  3. 테스트 코드에서 사용하는 word.txt파일이 따로 있어야 겠다는 생각이 들었습니다. (와잼님의 아이디어 차용)
    제가 원하는 단어를 테스트하기 위해서는 좋은 방법이 되었습니다.

  4. 그러다 보니 Wordle Game의 객체가 TodayWord를 밖에서 넘겨받는 형태가 아닌
    gameStartDate으로 게임을 시작한 날짜만 파라미터로 넘겨주면 되어
    WordleGameController까지 TodayWord도메인을 꺼내지 않아도 테스트가 되는 이점이 생겼습니다.
    또한, 게임을 시작한 날짜와 게임을 종료한 날짜가 다를 시
    게임을 시작한 날짜 기준으로 오늘의 단어를 만들고 정답 비교도 해당 오늘의 단어로 비교하는 시나리오처럼 보였습니다.

  5. WordleGameLogic에 여러가지 경우의 수의 테스트코드를 작성 하였습니다.
    테스트 코드로 프로덕션 코드를 보호한 후 리팩토링 하였는데 생각지 못한 테스트 실패가 생겼습니다.
    리팩토링 한 코드는 TodayWord가 아닌 WordComparator의 Letters로 비교하다 보니 생긴 문제였습니다.
    PRESENT(노란색)의 일치를 비교할 때에 기존 로직에서 CORRECT(초록색) 상태가 아닌 글자임을 하나 더 체크해야했습니다.

  6. Wordle Game을 도메인 영역이었다가 응용 영역으로 변경하였습니다.
    도메인들을 조합하는 역할을 하고 있었고 View를 위한 로직들도 혼재하게 되는 영역이 되었더라고요.
    Controller로 빼는 고민도 하였지만, 도메인 로직들이 Controller까지 가는건 아닌 것 같아 응용영역에 두게 되었습니다.

  7. 실제 테스트 코드도 도메인에 해당하는 단위테스트를 진행하기 보다 콘솔 출력에 의존해야 했습니다.
    유스님이 잘 작성해주신 테스트코드 덕분에 콘솔 출력 테스트코드는 처음 경험해보았는데 다양한 테스트코드 방법을 볼 수 있어 좋았습니다.
    다만, 너무 긴 출력때문에 출력부는 Fixture로 분리하고 싶었고, 해당 테스트 코드 맨 아래에 private 변수로 만들었습니다.

마지막으로 나의 느낀점

혼자서 다시 리팩토링 하는데 이틀이나 소요되었습니다.
제가 좀 느리고 생각이 많은 것 같습니다.
하지만 페어프로그래밍을 진행하면서 제가 덜 이해하고 지나간 부분을 다시 체크하는 과정을 경험하였습니다.

  1. 용어나 기능목록이 페어프로그래밍을 진행하면서 업데이트 된 부분과 누락된 부분을 명확히 쓰며 코드 구현부를 재정립하였습니다.
  2. 페어 할때는 유스님을 향해 감탄만 했다면 혼자 다시 코드 작성하면서 아 이래서 이 코드를 쓸수 밖에 없구나를 이해하는 시간을 가졌습니다. 이제 이 코드는 제겁니다. 😉
  3. TDD를 해보기 위해 모든 코드를 백지화(새출~) 하는 것이었습니다.
    테스트 코드 작성후 프로덕션 코드를 작성하니 시간이 오래걸렸지만 이미 작성된 테스트 코드 덕분에 프로덕션 코드의 신뢰도가 높아졌습니다.
  4. 커밋되기 전에 Ktlint와 테스트코드를 작동되게 하여 git에 올라가는 코드에 대해 커밋 기록 단위로 안정성을 보장하며 커밋하였습니다.
  5. 회고를 자세하게 적으니 또 새록새록 기억이 나면서 저의 무지함과 부족함을 확인하게 되었습니다.
    또한 유스님께 무한 감사 또또 합니다. 저는 인복이 있는 사람이라 그런지 항상 좋은 사람들과 일하게 되는 것 같습니다.🍀
  6. 아직도 부족한 부분이 많습니다.
    코틀린을 잘 안다고 할 수도 없고, 객체지향을 잘 안다고 할 수 없고, 테스트코드를 잘 작성했다고 할 수 없습니다.
    다음 리뷰해주시는 분께 좋은 리뷰를 부탁드립니다. 🙏🏻
  • 길고긴 페어프로그래밍 회고 이만 줄입니다.

boradol added 30 commits June 17, 2024 03:10
1. 세 가지 요구사항에 대해 간략하게 작성
2. 진행사항 중 페어프로그래밍 내용 작성
0. 기존 프로그래밍 요구 사항에서 수정
1. 환경
2. 구현 전 확인 사항
3. 구현 중 필요 사항
4. 제출 전 확인 사항
Solved Unknown Kotlin JVM target: 21

코틀린 1.9.0에서 JDK21을 지원하지 않아 생긴 문제
(gradle/gradle#25574 (comment))
1. 단어는 공백만 입력할 수 없다.
2. 단어의 길이는 5자 이어야 한다.
1. DictionaryFileLoader에서 단어목록 불러옴.
2. 사전에서 단어목록 포함여부를 판별한다.
1. WordleExceptionCode에서 메시지를 관리한다.
2. 이 에러 메시지는 게임에서 다시시도 이유 메시지로 활용될 수 있다.
1. 알파벳 소문자를 생성한다.
2. 일치 표시 기호를 생성한다.
3. 예외 케이스 테스트 작성한다.
1. 단어 목록에 관련된것은 DictionaryFileLoader에서만 관리하기
1. 사전의 단어목록에서 오늘의 날짜에 맞게 가져온다.
2. 오늘의 단어는 매일 바뀐다.
1. 게임 시작하는 메시지 출력
2. 답안 문자를 입력받는 문구 출력후 문자열 입력받기
1. Application : 프로그램 실행
2. WordleGameController : 워들 게임을 시작 할 수 있게 하는 시작 점
1. 글자 일치 상태 Enum 추가
2. Word클래스가 List의 인터페이스를 구현하도록 변경
3. 여러가지 상황들 테스트케이스 작성
4. 테스트에 필요한 단어 몇 개 추가
1. 단어 결과는 각 글자의 글자 일치 여부 상태를 변경할 수 있다.
2. WordleGameLogic에서 비교한 단어들의 각 글자의 일치 상태를 관리한다.
1. 테스트 코드는 WordGameLogic 테스트가 통과하는지 확인한다.
2. 단어 비교기는 오늘의 단어에 일치 표시 기호를 하여 일치 비교를 완료한 글자를 관리한다.
1. 단어결과를 추가하여 게임의 진행여부와 게임 성공여부를 판단할 수 있다.
1. 게임 결과 출력
2. 타일 그리기
3. 다시시도 메시지
4. 성공 메시지
5. 실패 메시지, 오늘의 단어 안내
Copy link

@cousim46 cousim46 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코틀린에 대해 많이 알지는 못했지만 보라돌님 코드 덕분에 새로운 문법들을 알 수 있는 기회였던거 같습니다! 해당 과제를 구현하시느라 정말 고생많으셨습니다! 만약 의아한 부분, 제가 잘못 생각한 부분이 있다면 편하게 말씀해주시면 감사하겠습니다! 고생하셨습니다!

Comment on lines +6 to +8
GREEN(LetterMatch.CORRECT, "🟩"),
YELLOW(LetterMatch.PRESENT, "🟨"),
GREY(LetterMatch.ABSENT, "⬜"),

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오.. 이 부분을 enum을 활용하셨네요..!

Copy link
Author

@boradol boradol Jun 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

감사합니다!! ☺️

Comment on lines +18 to +29
fun `(성공) 오늘의 단어는 매일 바뀐다`() {
val criterionDate = LocalDate.of(2024, 6, 17)
val wordsSet = mutableSetOf<Word>()

repeat(365) {
val currentDate = criterionDate.plusDays(it.toLong())
val word = TodayWord(currentDate)
wordsSet.add(word)
}

assertThat(wordsSet).hasSize(365)
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이거에 대해서 테스트를 해볼 생각을 한적이 없는데 이것까지 생각하고 테스트 코드를 짜셨네요..! 👍

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

요구사항에 오늘의 단어는 매일 바뀌며라는 부분이 있어 테스트를 하게 되었는데요 ㅠ
사실 이부분을 이렇게 테스트 하는 것이 맞나 생각하엿는데 호이스토리님께서 알아봐주셔서 기분 좋네요!! 🤗


fun isContinuousGame(): Boolean = !isSuccessfulGame() && tryCount.isRemainder()

fun isSuccessfulGame(): Boolean = (tryCount.attempts != 0) && results.last().isSuccessfulWordResult()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tryCount.attempts != 0

이 부분에서 ()가 필요한 이유가 따로 있으실까요??

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

사실 있으나 없으나 상관은 없으나
가독성면에서 앞에 문장안에 연산자가 있어 괄호를 치는것이 더 낫다 생각했어요.
아니면 private 함수로 빼도 될것 같고요.

private fun isPresentLatter(
index: Int,
answerLetter: Letter,
) = !(wordResult.isCorrectLetterMatch(index) || isCorrectLetter(index, answerLetter)) && (answerLetter in markerLetters)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

 !(wordResult.isCorrectLetterMatch(index)

해당 부분에 () 가 필요한 이유를 알 수 있을까요?!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

!( wordResult.isCorrectLetterMatch(index) || isCorrectLetter(index, answerLetter) )

묶어서 표현했습니다.
이부분이 CorrectLetter인지 아닌지 Validation이 필요한 로직이라서요.
그럼 이 부분을 묶은것은 어떻게 생각하세요?
이것도 따로 private 함수로 빼려다가 내부 함수의 뎁스가 너무 깊어질거 같아서 그냥 이렇게 두었습니다.

Comment on lines +24 to +30
@ValueSource(strings = [" ", "1", "A", "나", "@"])
@ParameterizedTest
fun `(예외) 유효하지 않은 글자를 생성하면 예외 발생한다`(letter: Char) {
assertThatThrownBy { (Letter(letter)) }
.isInstanceOf(IllegalStateException::class.java)
.hasMessage("유효하지 않은 글자 형식입니다.")
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

해당 부분에 대문자도 예외를 발생시켜주는거 같습니다!
만약 오늘 문자의 답이 apple일 경우 사용자가 APPLE를 입력하면 예외가 발생하는 부분일까요!
저는 사용자가 대문자로 입력을 하든 소문자로 입력을 하든 같은 단어이면 같은 의미를 가지고 있는데 예외를 발생시키는것보다는 대문자로 입력이 들어올 경우 소문자로 변환을 해주고 비교를 해주는 방식이 있다고 생각하는데 어떻게 생각하시나요!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

네네 맞습니다.
저희도 정책을 어떻게 정할까 고민한 부분이었는데 잘 말씀해주셨네용! 😄

저희는 논의 하고 위에 용어사전이나 기능목록에 소문자만 입력받는 것으로 명시하고
소문자만 입력하는 것으로 구현해보기로 하였습니다.

대문자로 입력이 들어올 경우 소문자로 변환을 해주고 비교를 해주는 방식에 대해
ignoreCase 같은것으로 변환해 볼 수 있는 방법이 있을 것 같아
요구사항에 따라 바꾸는 것은 어렵지 않다고 페어분과 이야기 나누고 진행하게 되었습니다. 😀

const val WORD_LENGTH = 5

fun Word(word: String): Word {
check(word.isNotBlank()) { WORD_NOT_ALLOW_SPACE.message }

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여태 require만 알고있었는데 덕분에 새로운거 알게되었습니다! 감사합니다!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

네네 다시 시도시에 IllegalStateException이 더 알맞아서 check를 사용하게 되었어용!

Comment on lines +9 to +14
fun `(성공) 오늘의 단어는 오늘 날짜를 입력받아 단어를 생성한다`() {
val today = LocalDate.of(2021, 6, 19)

val actual = TodayWord(today)

assertThat(actual).isEqualTo(Word("hello"))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오늘의 답에 대한 테스트 코드로 작성하기 애매했는데 보라돌님이 작성해주셨네요..!
해당 테스트에 대해서 궁금한 사항이 있습니다!
궁금한 사항1 : 만약 words.txt의 단어들이 바뀌게 된다면 해당 테스트 코드는 실패하게 될거 같은데 맞을까요?
궁금한 사항2 : 만약 실패를 하게 된다면 어떻게 수정을 해야 해당 테스트 코드를 성공시킬지 궁금합니다.

만약 제 질문이 이상하거나 잘못된 질문일 경우는 말씀해주시면 감사하겠습니다!! :)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

궁금한 사항1 : 만약 words.txt의 단어들이 바뀌게 된다면 해당 테스트 코드는 실패하게 될거 같은데 맞을까요?

따로 test/resources/경로 아래 words.txt 파일을 만들어 두어서 테스트용으로만 사용할 수 있게 구현되어 있습니다.
그래서 프로덕션과 영향이 있지 않은데요.
혹시 테스트용이 변경될 것을 말씀하시는 것일까용?

궁금한 사항2 : 만약 실패를 하게 된다면 어떻게 수정을 해야 해당 테스트 코드를 성공시킬지 궁금합니다.

위에 답변이 같은 대답이 될 수 있을지는 모르겠지만,
테스트용으로 파일을 만들었기 때문에 테스트 안에서 실패가 나지 않게
test/resources/words.txt파일 변경에 주의를 기울여야 할 것 같습니다.

Comment on lines 6 to 18
init {
check(isAlphabetOrMatchMarker()) { LETTER_INVALID_CHARACTER_TYPE.message }
}

fun changeMatchMarker(): Letter = MATCH_MARKER_LETTER

fun value(): String = value.toString()

private fun isAlphabetOrMatchMarker(): Boolean = isAlphabet() || isMatchMarker()

private fun isAlphabet(): Boolean = value in ALPHABET

private fun isMatchMarker(): Boolean = value == MATCH_MARKER

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

private fun isAlphabetOrMatchMarker(): Boolean = isAlphabet() || isMatchMarker()

해당 메서드는 isAlphabet(), isMatchMarker()를 한번에 조합하여 만든 함수인거 메서드인거 같습니다.

그런데

init {
        check(isAlphabetOrMatchMarker()) { LETTER_INVALID_CHARACTER_TYPE.message }
    }

해당 부분에서 isAlphabetOrMatchMarker()를 사용하셨습니다.
해당 메서드는 Letter.kt 내부에서만 사용되고 있고 다른곳에서는 사용 안하고 있는걸로 인지하고 있습니다.
여기서 궁금한점은 isAlphabetOrMatchMarker 해당 메서드를 Letter.kt 파일에서만 사용되고 있는데 init에서 아래와 같이 사용하면 isAlphabetOrMatchMarker 메서드를 따로 안만들수 있었을텐데 따로 만들어서 사용하신 이유가 있으실까요!!?

init {
        check(isAlphabet() || isMatchMarker()) { LETTER_INVALID_CHARACTER_TYPE.message }
    }

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

init 블록안에 되도록 ||&&를 넣지 않으려고 해서 그렇습니다 🤗
하지만 호이스토리님이 제안해주신 부분도 좋은 코멘트라고 생각합니다.

이런 부분에서
함수를 덜 만드는 것과 함수 이름에서 이미 or의 의미를 쓰고 있는 부분에서 호이스토리님이 말씀해 주신 부분이 더 좋다고 생각이 듭니다. 수정해서 반영하겠습니다!

Comment on lines +8 to +12
private val dictionaryWords: List<String> by lazy { loadDictionaryWords() }
private val dictionaryWordSet: Set<String> by lazy { dictionaryWords.toSet() }
val dictionaryWordsSize = dictionaryWords.size

fun contains(word: String): Boolean = dictionaryWordSet.contains(word)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dictionaryWords(List) -> dictionaryWordSet(Set) 으로 변환을 하여 dictionaryWordSet.contains(word) 단어 포함 여부를 나타내는거 같습니다!

dictionaryWords -> dictionaryWordSet으로 변환을 해서 얻을 수 있는 이점은 중복 제거라고 생각이 됩니다(만약 아니면 말씀해주세요!)

여기서 궁금한점은 중복 제거를 안하더라도 dictionaryWords를 이용하여 단어가 포함되어있는지 비교할 수 있을거 같은데 왜 따로
dictionaryWordSet을 만들어서 단어 포함여부를 비교하는지 궁금합니다!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dictionaryWordSet으로 변환의 이점은
시간복잡도 면에서도 이득을 볼 수 있어 이렇게 구현해보았습니다.🥲

해당dictionaryWords에서 해당 단어가 contains인지 판별할 때
List는 O(n) 복잡도, Set O(1) 복잡도로 활용할 수 있습니다.
그래서 성능상 contains가 자주 사용될 것 같았고, 조금더 성능을 높여주면 좋을 것 같아 변환 시켜주었습니다.

또한, 오늘의 단어를 할 때는 List의 index로 접근해야할 것 같아서 일단 이렇게 두가지를 사용해보았습니다.

@Test
fun `6번 이내 오늘의 단어를 맞추면 성공 안내 문구를 출력한다`() {
val inputAnswerWords = successConsoleInput
val (printStream, outputStream) = printStreamByteArrayOutputStreamPair(inputAnswerWords)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오.. 제가 생각하는 코틀린의 장점 중 하나라고 생각하는 구조 분해 방식을 사용하셨네요!(만약 아니라면 죄송합니다 ㅠㅠ)
저도 이 개념을 알고있음에도 불구하고 언제 어떻게 활용을 해야될지 감을 못잡았었는데 보라돌님 코드 덕분에 언제 어떻게 활용해야될지 조금 감이 잡히는거 같습니다! 감사합니다 :)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

네네 페어분이랑 코틀린스럽게... 해보려고 시도를 많이 해봤습니다.
이렇게 하니까 아래 변수로도 활용할수 있고 좋더라고요.

저도 인텔리제이에서 함수 만드는 단축키(cmd + option + m) 사용하다가 만들어진 함수여서
우연히 발견하게 되었는데 되게 좋더라고용!! >.<

Copy link
Author

@boradol boradol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

호이스토리님 >_<
좋은 리뷰 감사합니다!
호이스토리님께 피드백을 받다니 영광입니다.🥹🥹🥹🥹🥹

제가 여행중이어서 조금 늦은 코멘트를 남기게 되었어요!
피드백 주신 부분에서
반영한 부분은 새로 Push하였습니다.

그리고 중간에 의견 나누고싶은 부분도 제가 코멘트 다시 남겼어용!
많이 부족한 지식이지만 더 얘기 나눌 부분이나 혹시 저에게 부족한 부분이 있으면
또 좋은 의견 많이 남겨주세요!!!

감사합니다 _

Comment on lines 6 to 18
init {
check(isAlphabetOrMatchMarker()) { LETTER_INVALID_CHARACTER_TYPE.message }
}

fun changeMatchMarker(): Letter = MATCH_MARKER_LETTER

fun value(): String = value.toString()

private fun isAlphabetOrMatchMarker(): Boolean = isAlphabet() || isMatchMarker()

private fun isAlphabet(): Boolean = value in ALPHABET

private fun isMatchMarker(): Boolean = value == MATCH_MARKER
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

init 블록안에 되도록 ||&&를 넣지 않으려고 해서 그렇습니다 🤗
하지만 호이스토리님이 제안해주신 부분도 좋은 코멘트라고 생각합니다.

이런 부분에서
함수를 덜 만드는 것과 함수 이름에서 이미 or의 의미를 쓰고 있는 부분에서 호이스토리님이 말씀해 주신 부분이 더 좋다고 생각이 듭니다. 수정해서 반영하겠습니다!

const val WORD_LENGTH = 5

fun Word(word: String): Word {
check(word.isNotBlank()) { WORD_NOT_ALLOW_SPACE.message }
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

네네 다시 시도시에 IllegalStateException이 더 알맞아서 check를 사용하게 되었어용!

private fun isPresentLatter(
index: Int,
answerLetter: Letter,
) = !(wordResult.isCorrectLetterMatch(index) || isCorrectLetter(index, answerLetter)) && (answerLetter in markerLetters)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

!( wordResult.isCorrectLetterMatch(index) || isCorrectLetter(index, answerLetter) )

묶어서 표현했습니다.
이부분이 CorrectLetter인지 아닌지 Validation이 필요한 로직이라서요.
그럼 이 부분을 묶은것은 어떻게 생각하세요?
이것도 따로 private 함수로 빼려다가 내부 함수의 뎁스가 너무 깊어질거 같아서 그냥 이렇게 두었습니다.


fun isContinuousGame(): Boolean = !isSuccessfulGame() && tryCount.isRemainder()

fun isSuccessfulGame(): Boolean = (tryCount.attempts != 0) && results.last().isSuccessfulWordResult()
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

사실 있으나 없으나 상관은 없으나
가독성면에서 앞에 문장안에 연산자가 있어 괄호를 치는것이 더 낫다 생각했어요.
아니면 private 함수로 빼도 될것 같고요.

Comment on lines +8 to +12
private val dictionaryWords: List<String> by lazy { loadDictionaryWords() }
private val dictionaryWordSet: Set<String> by lazy { dictionaryWords.toSet() }
val dictionaryWordsSize = dictionaryWords.size

fun contains(word: String): Boolean = dictionaryWordSet.contains(word)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dictionaryWordSet으로 변환의 이점은
시간복잡도 면에서도 이득을 볼 수 있어 이렇게 구현해보았습니다.🥲

해당dictionaryWords에서 해당 단어가 contains인지 판별할 때
List는 O(n) 복잡도, Set O(1) 복잡도로 활용할 수 있습니다.
그래서 성능상 contains가 자주 사용될 것 같았고, 조금더 성능을 높여주면 좋을 것 같아 변환 시켜주었습니다.

또한, 오늘의 단어를 할 때는 List의 index로 접근해야할 것 같아서 일단 이렇게 두가지를 사용해보았습니다.

Comment on lines +6 to +8
GREEN(LetterMatch.CORRECT, "🟩"),
YELLOW(LetterMatch.PRESENT, "🟨"),
GREY(LetterMatch.ABSENT, "⬜"),
Copy link
Author

@boradol boradol Jun 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

감사합니다!! ☺️

Comment on lines +24 to +30
@ValueSource(strings = [" ", "1", "A", "나", "@"])
@ParameterizedTest
fun `(예외) 유효하지 않은 글자를 생성하면 예외 발생한다`(letter: Char) {
assertThatThrownBy { (Letter(letter)) }
.isInstanceOf(IllegalStateException::class.java)
.hasMessage("유효하지 않은 글자 형식입니다.")
}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

네네 맞습니다.
저희도 정책을 어떻게 정할까 고민한 부분이었는데 잘 말씀해주셨네용! 😄

저희는 논의 하고 위에 용어사전이나 기능목록에 소문자만 입력받는 것으로 명시하고
소문자만 입력하는 것으로 구현해보기로 하였습니다.

대문자로 입력이 들어올 경우 소문자로 변환을 해주고 비교를 해주는 방식에 대해
ignoreCase 같은것으로 변환해 볼 수 있는 방법이 있을 것 같아
요구사항에 따라 바꾸는 것은 어렵지 않다고 페어분과 이야기 나누고 진행하게 되었습니다. 😀

Comment on lines +9 to +14
fun `(성공) 오늘의 단어는 오늘 날짜를 입력받아 단어를 생성한다`() {
val today = LocalDate.of(2021, 6, 19)

val actual = TodayWord(today)

assertThat(actual).isEqualTo(Word("hello"))
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

궁금한 사항1 : 만약 words.txt의 단어들이 바뀌게 된다면 해당 테스트 코드는 실패하게 될거 같은데 맞을까요?

따로 test/resources/경로 아래 words.txt 파일을 만들어 두어서 테스트용으로만 사용할 수 있게 구현되어 있습니다.
그래서 프로덕션과 영향이 있지 않은데요.
혹시 테스트용이 변경될 것을 말씀하시는 것일까용?

궁금한 사항2 : 만약 실패를 하게 된다면 어떻게 수정을 해야 해당 테스트 코드를 성공시킬지 궁금합니다.

위에 답변이 같은 대답이 될 수 있을지는 모르겠지만,
테스트용으로 파일을 만들었기 때문에 테스트 안에서 실패가 나지 않게
test/resources/words.txt파일 변경에 주의를 기울여야 할 것 같습니다.

Comment on lines +18 to +29
fun `(성공) 오늘의 단어는 매일 바뀐다`() {
val criterionDate = LocalDate.of(2024, 6, 17)
val wordsSet = mutableSetOf<Word>()

repeat(365) {
val currentDate = criterionDate.plusDays(it.toLong())
val word = TodayWord(currentDate)
wordsSet.add(word)
}

assertThat(wordsSet).hasSize(365)
}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

요구사항에 오늘의 단어는 매일 바뀌며라는 부분이 있어 테스트를 하게 되었는데요 ㅠ
사실 이부분을 이렇게 테스트 하는 것이 맞나 생각하엿는데 호이스토리님께서 알아봐주셔서 기분 좋네요!! 🤗

@Test
fun `6번 이내 오늘의 단어를 맞추면 성공 안내 문구를 출력한다`() {
val inputAnswerWords = successConsoleInput
val (printStream, outputStream) = printStreamByteArrayOutputStreamPair(inputAnswerWords)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

네네 페어분이랑 코틀린스럽게... 해보려고 시도를 많이 해봤습니다.
이렇게 하니까 아래 변수로도 활용할수 있고 좋더라고요.

저도 인텔리제이에서 함수 만드는 단축키(cmd + option + m) 사용하다가 만들어진 함수여서
우연히 발견하게 되었는데 되게 좋더라고용!! >.<

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants