[Wordle] 돌쓰팀(yooth) 미션 제출합니다. #20
Conversation
게임 단어에 포함될 글자 하나를 담는 값 객체
Letter 객체 생성 검증 테스트
오늘의 단어, 답안 단어를 담는 클래스
Word 객체 생성 검증 테스트
매일 단어가 변경 되는지 검증
| import wordle.exception.ExceptionMessage | ||
| import wordle.loader.isContainsDictionaryWord |
There was a problem hiding this comment.
요기를 보시면 도메인인데 외부를 의존하고 있네요
저는 보통 도메인은 다른것에 의존성이 없게 짜려고 노력하는편인데 어떻게 생각하시는지 궁금합니다~
There was a problem hiding this comment.
제가 계층 간 의존에 대한 개념이 조금 부족한데 어떻게 풀어야 할지 고민해 보겠습니다!
| import wordle.loader.isContainsDictionaryWord | ||
| import java.time.LocalDate | ||
|
|
||
| typealias TodayWord = Word |
There was a problem hiding this comment.
정답단어과 입력단어에서 클래스분리 대신 중도를 찾으셨군요ㅋㅋ
적절한 활용인것 같습니다~ 💯
| } | ||
|
|
||
| fun TodayWord(today: LocalDate): TodayWord { | ||
| return Word(extractWordleWord(today)) |
There was a problem hiding this comment.
저는 네임스페이스처럼 나오는게 보기편해서 확장함수가 아니면 탑레벨에 선언하지 않고 클래스안에 넣어두는 편인데요
IDE 나 깃헙콘솔로 들어오지 않으면 extractWordleWord 가 어딨는지 찾기 힘든 단점도 있네요
탑레벨에 선언하는걸 선택하는데는 어떤 장점들이 있어서 결정하셨는지 궁금합니다!
There was a problem hiding this comment.
저는 개인적으로 코틀린의 가짜 생성자 패턴을 좋아하는데 이번 미션 하면서 이 내용을 녹여보고 싶었습니다!
작업 하면서도 '파일을 분리하는 것이 좋나?' 에 대한 고민은 있었지만 파일이 너무 많아질 것 같아서 같은 파일 안에 함수를 두었습니다.
There was a problem hiding this comment.
아하 저는 extractWordleWord 이 함수가 클래스없이 탑레벨에 선언된거에 대해서
제가 적은 단점에 비해 어떤 장점이 있는지 궁금해서 코멘트를 단거였는데요
위치가 부적절해서 오해여지가 있었군요 죄송합니다ㅎ
|
|
||
| const val WORD_LENGTH = 5 | ||
|
|
||
| fun Word(word: String): Word { |
There was a problem hiding this comment.
외부에서 Word(listOf<Letter>()) 처럼 유효성체크없이 생성이 가능하겠네요
주생성자를 private 으로 막고 동반객체에 팩터리메서드를 만들어볼수 있을것같아요! (그러고보니까 저도 빼먹었네요ㅋㅋ)
There was a problem hiding this comment.
주 생성자에 접근 제어자를 추가 해야 했는데 급하게 제출하느라 놓쳤네요ㅠㅠ
There was a problem hiding this comment.
궁금해서 해봤는데 주생성자를 private으로 막으면 가짜 생성자를 사용할수가 없네요
internal 이 최선인것같아요
| fun minus() { | ||
| check(isRemainder()) { ExceptionMessage.TRY_COUNT_HAS_NOT_REMAINDER.message } | ||
| --count | ||
| } |
|
|
||
| assertThatThrownBy { tryCount.minus() } | ||
| .isInstanceOf(IllegalStateException::class.java) | ||
| .hasMessage("시행 횟수는 0보다 작을 수 없습니다.") |
There was a problem hiding this comment.
TRY_COUNT_HAS_NOT_REMAINDER 를 깜박하신것같네요!
There was a problem hiding this comment.
저희는 테스트 검증 단계에선 프로덕션 코드에 가능한 의존하지 않도록 테스트를 작성했습니다!
There was a problem hiding this comment.
저희는 테스트 검증 단계에선 프로덕션 코드에 가능한 의존하지 않도록 테스트를 작성했습니다!
의도하신바가 프로덕션코드 변경에 의해서 테스트 깨지는걸 최소화하려했다는 걸로 이해했는데 맞을까요?
근데 그렇다면 오히려 메시지변경에 의해 테스트 코드가 깨질 확률이 더 높지 않을까요?
발생한 예외 정보에 대해서 프로덕션 코드에 의존하지 않으려면 예외생성 행위를 모킹해서 해당테스트에서 메시지를 정의해야 의미가 있지 않았을까 생각해봅니다! 😃
There was a problem hiding this comment.
이 부분에서는 제가 이 부분을 지켜보는 것에 대해 좀 강력하게 얘기를 한 부분이어서 그런 것 같아요. :)
저도 아직 이 부분에 대해서 좀 어색한 감이 있는데요.
어떤 리뷰어님께서 아티클을 공유해주신 것이 있어서 이것을 따라보고자 하여 이런 스타일이 나오게 되었습니다.
항상 회사 컨벤션에 맞춰 하는것이 항상 맞다고 생각하지만
저희가 이번 프로그래밍에 하드 코딩한 부분이 많은 이유에 대해 이해가 되실 것 같아서 해당 아티클 링크 첨부합니다!!
https://jojoldu.tistory.com/615
점프님 말씀대로
예외생성 행위를 모킹해서 해당테스트에서 메시지를 정의해야 의미가 있지 않았을까 생각해봅니다!
이 부분도 좋은 코멘트라고 생각합니다 !!
저 남의 PR와서도 많이 배우네용!!
|
|
||
| class TodayWordTest { | ||
| @Test | ||
| fun `오늘의 단어는 매일 바뀐다`() { |
There was a problem hiding this comment.
저는 단어가 매일바뀌는지 테스트보다 의도한 계산식대로 선택하는지 테스트하는게 더 요구사항을 잘 검증한다고 생각하는데 유스님은 어떠신가요?
There was a problem hiding this comment.
이 테스트에서 고민이 많았습니다ㅠㅠ
의도한 계산식이란건 words.txt 파일에 의존하게 돼서 좋은 테스트가 아니라고 느꼈던 것 같아요.
words.txt 파일에 단어가 추가/삭제 되면 테스트에 영향이 가게 돼서요.
테스트용 파일을 만드는 것은 고민했습니다.
There was a problem hiding this comment.
넵 말씀주신것처럼 개인적으론 테스트용 파일을 따로 만들었으면 더 좋았을것 같은데
이 방법을 선택하신건 이렇게도 충분히 검증가능하다 판단하셔서겠죠? 👍
| fun `단어의 모든 글자가 '불일치'하다`() { | ||
| val answerWord = Word("adapt") | ||
|
|
||
| val actual = WordComparator(오늘의_단어).matchCorrect(answerWord).matchPresent(answerWord) |
There was a problem hiding this comment.
위에 말씀드린거랑 비슷한 이야기인데 이렇게 "완전일치 먼저검사하고 부분일치 검사를 해야한다"라는 도메인지식이 중복되는 단점도 있는것같아요!
There was a problem hiding this comment.
게임 알고리즘 상 어쩔수 없는 부분이라 생각하는데 점프님은 어떻게 해결하셨나요??
given 에서 matchCorrect() 를 호출하는 방법이라면 아래처럼 하는 방법과 동일할 것 같습니다!
| val actual = WordComparator(오늘의_단어).matchCorrect(answerWord).matchPresent(answerWord) | |
| // given | |
| val comparator = WordComparator(오늘의_단어).matchCorrect(answerWord) | |
| // when | |
| comparator.matchPresent(answerWord) | |
| // then | |
| assertThat(//....) |
There was a problem hiding this comment.
조금 기우일수도 있긴한데요ㅎㅎ
지금 WordComparator 에서 완전일치와 부분일치의 순서를 강제하지 않고 있기 때문에 다른 개발자가 WordGameLogic 을 거치지않고 바로 WordComparator().matchPresent().matchCorrect() 처럼 순서를 바꿔서 오용할 가능성이 있다고 생각했어요!
그리고 만약 여러 클래스에서 단어 비교를 위해서 WordComparator에 바로 접근한다면 WordComparator().matchCorrect(answerWord).matchPresent(answerWord) 해당 문장이 반복될 거고 그게 도메인로직(완전->부분 순으로 해야한다) 의 중복이라 생각했어요!
matchCorrect, matchPresent 를 외부에 노출하지 않고 WordComparator에 순서를 보장한 compare 메서드를 하나 만들것 같아요
| fun `단어에 '완전 일치'한 글자가 있다`() { | ||
| val answerWord = Word("harry") | ||
|
|
||
| val actual = WordComparator(오늘의_단어).matchCorrect(answerWord).matchPresent(answerWord) |
There was a problem hiding this comment.
이건 개취인데요 오늘의_단어가꼭 한번에 같이 바뀌어야할 이유가 없다면
그냥 테스트하는곳에 "hello" 라고 나오는게 읽기 더쉽다고 생각하는데 어떠신가요 😃
There was a problem hiding this comment.
저도 테스트의 given 절은 테스트 메서드 내부에 하드코딩 하는 것을 좋아하는데
페어분과 얘기 나눠보고 내린 결정입니다!
| fun `단어 결과목록이 게임 단어 길이와 일치하지 않으면 예외를 반환한다`() { | ||
| assertThatThrownBy { WordResult(mutableListOf()) } | ||
| .isInstanceOf(IllegalStateException::class.java) | ||
| .hasMessage("단어 비교 결과는 단어 길이(5)와 일치해야 합니다.") |
There was a problem hiding this comment.
INVALID_WORD_RESULT_LENGTH 요거 사용가능할것 같습니다!
There was a problem hiding this comment.
위에서 말씀 드린 내용처럼 검증부에서 프로덕션 코드의 의존을 가능하면 줄이고자 하는 시도였습니다!
보라돌님 덕분에 많은 내용 배웠습니다!
객체에게 적절한 역할과 책임을 부여하는게 어려웠고
진행할수록 무한한 추상화를 하고 그 추상을 의존하려는 버릇이 있었습니다.
그로인해 객체의 불필요한 의존이 늘어나게 되고 이를 적절히 끊어주는 연습이 필요하다 느꼈습니다.
테스트 코드는 여전히 어려웠습니다ㅠㅠㅠ
린트를 맞추는 경험도 해본적이 없었는데 공통된 규칙이 정해진 느낌이라 확실히 좋다 느꼈습니다!
이 내용은 회사 출근해서 바로 팀원들에게 공유 했습니다.
지금까지 사를 만난 경험이 없었는데, 보라돌님이 이번 미션에서 사수 역할을 해주셔서 너무 좋은 경험이었습니다!