[5기] 3주차 Wordle 과제 제출 - 구월 #33
Conversation
There was a problem hiding this comment.
안녕하세요. 구월님 >_<
제가 구월님의 리뷰를 맡게 되었네요!
사실 너무나 훌륭한 코드라서
제가 감히 여기에 리뷰를 남긴다는 것이 잘 완성된 코드에 이상한 리뷰어 빠뜨리기 같은 느낌이었어요
🤣🤣🤣🤣🤣🤣🤣
그래도 저도 또 다른사람의 코드를 보며 공부한다 생각하였고,
저희 팀이 구현하면서 고민했던 부분들도 함께 나눌 수 있겠다 생각하여
열심히 코드를 살펴보았습니다!
일단 우지님이랑 페어 프로그래밍 하실 때 두분다 너무 독보적이셔서
사실 코드 구현부 보고 정말 잘 설계했다 라는 생각이 들었습니다.
좋은점은 일단 구월님께서 워낙 깔끔하게 코드를 구현하시고 클래스 분리를 잘 하시고
객체지향이란 이거다!!! 라는 것을 보여준 코드라 생각해요!
메서드 명이나 딱 직관적이라서 이해하기 쉬운 코드였어요!
그리고 클래스 다이어그램 부분에서도 엄청난 성의가 보여지는 고퀄리티 과제였다고 생각합니다.🎉
구월님 코드 보면서 저의 부족한 코드가 너무 많이 보여서 사실 제가 리뷰를 해도 되나 싶을 정도였습니다.
그리고 제가 코멘트 남긴부분은
- 의견을 묻고싶거나
- 제가 궁금해서 의견을 묻고싶은 부분
- 미비한 실력이지만 얄팍한 지식으로 알고있는 부분에 대해 도움이 될 수 있는 부분
을 추가로 코멘트 남기게 되었어요!
제가 잘 모르고 얘기한 부분이나
혹시 제 이야기에 의견 공유해주실 부분 있으면 언제든 말씀해주세요!
그리고 구월님의 코드를 리뷰할 수 있어서 영광이었습니다!!!🙇🏻♀️
제가 지금 여행중이라서 새벽에 리뷰를 남기느라 조금 정신줄 놓고 코멘트 남긴 부분이 있을수도 있어여ㅠ🫠
그래서 좀 이상한 말 있으면 ㅠㅠ 꼭 말씀해주세요!!
아무튼 페어프로그래밍 하면서 정말 개발 인생으로 재미있고 색다른 경험을 해보게 되엇는데
이렇게 리뷰어까지 해볼 수 있어 감회가 새롭네요!
더구나 제가 굉장히 좋아하는 구월님 것을 리뷰할 수 있어서 좋았습니다.💝
아무튼 긴 글 읽어주셔서 감사합니다.
구월님 마지막으로 3차 과제 하느라 고생 많으셨어요! 우린 이제 짱짱의 길을 걸을것이에요! 🙌
|
|
||
| import java.util.Arrays; | ||
|
|
||
| public enum Alphabet { |
| this.wordListReader = wordListReader; | ||
| } | ||
|
|
||
| public void start(final WordSelector wordSelector) { |
| } | ||
|
|
||
| private Result examine(final WordList wordList, final Answer answer) { | ||
| final Guess guess = inputWord(wordList); |
There was a problem hiding this comment.
inputWord메서드에 파라미터에 wordList가 들어가는 것이 조금 부자연스럽게 느껴집니다.
제생각에는 wordList는 GuessWord가 포함되었는지 validation하려고 넘기는 것이라 생각이 드는데요.
뭔가 input이라는 메서드가 다른 행동도 하고 있다 생각이 드는데요.
메서드 명에 대해 어떻게 생각하세용? 🥲
There was a problem hiding this comment.
확실히 inputWord 메소드가
단어를 입력받는 역할과 입력받은 단어의 validation을 하는 역할 두 가지를 수행하고 있는 것으로 보이네요.
좋은 지적 감사합니다!
메서드 네이밍을 더 명확하게 하거나, 각 역할별로 메서드를 분리하는 방향으로 고민해서 다시 반영해보겠습니다!
| outputView.insertWord(); | ||
| final Word word = inputView.inputWord(); | ||
| return new Guess(wordList.getWordIfExists(word)); | ||
| } catch (final Exception e) { |
There was a problem hiding this comment.
다시 시도하기 위한 try-catch문이네용!
근데 Exception으로 잡게되면,
다시 시도하고 싶어하는 예외들 말고도
다른 예외 발생시(IOException 같은...)에도 다시시도가 되지 않을까요?
혹시 다시 시도하고 싶은 경우가 어떤 경우였을까요?🥹
There was a problem hiding this comment.
처음에 Exception을 명확히 정의하지 않은 상태에서 코드를 짰다가
Exception을 정의한 후 수정하는 것을 깜빡했네요... 🥲
Guess를 생성할 때의 validation을 통과하지 못하거나 wordList에 없는 단어가 들어오는 경우에
다시 시도하는 것으로 생각했습니다!
이 부분도 수정하여 반영할게요!
| - **기능 요구 사항에 기재되지 않은 내용은 스스로 판단하여 구현한다.** | ||
| - 참여자 : 우지, 구월 | ||
|
|
||
| ## 시퀀스 다이어그램 |
There was a problem hiding this comment.
시퀀스 다이어그램 포함하느라 고생많으셨어요!
전체적인 맥락이 보여서 정말 좋았어요!!! 👍
There was a problem hiding this comment.
우지님이 만드시고 제가 살짝 수정만 한 다이어그램입니다!
저도 처음에 보고 화들짝 놀랐답니다 👍
| public class AnswerTest { | ||
|
|
||
| @Test | ||
| void 정답은_정답과_동일한_답안이_들어오면_모두_MATCHED인_결과를_반환한다() { |
There was a problem hiding this comment.
정말 잘 된 로직이라 생각해요!
제가 여러가지 케이스 넣어 검증해봤는데 정말 최고입니다.
그래서 MATCHED말고도 여러가지 테스트 케이스가 있으면,
혹시나 나중에 프로덕션코드가 변경되더라도 무적방어가 가능할 것 같아용!! 🛡️
There was a problem hiding this comment.
말씀주신대로 MATCHED 이외에 다양한 테스트케이스를 추가해보겠습니다!
핵심 로직에 대한 테스트인지라 상세한 케이스를 추가할수록 불확실성을 줄일 수 있을 것 같아요 👍
| } | ||
|
|
||
| @Test | ||
| void 정답은_5글자가_아니면_예외를_발생한다() { |
There was a problem hiding this comment.
Word에서 하는 검증을 Answer, Guess에서도 하고 있는 것 같아요.
근데 실제 validation이 행해지는 곳은 Word라 생각해요!
물론 두번 체크하는 것은 나쁘지 않다고 생각합니다!!
하지만
예를 들어 Answer에서만 잡고싶은 예외보다 Word에서 잡는 예외가 먼저 터질 때,
서로 다른 Exception으로 잡고 있다면, 테스트 케이스가 깨지는 경우를 경험한 적이 생각나더라구용 ㅠㅠㅠㅠ
또는 Word의 Exception을 다른 CustomException으로 바꾸게 되면 다 변경해야할 수도 있습니다.
이런 경우도 있을 것 같아서 코멘트 남기게 되었어요.
하지만 구찌팀에서 잘 판단하여 작성하셨을거라 생각이 들어용!!! >_<
There was a problem hiding this comment.
말씀해주신대로 Answer와 Guess에서 완전히 동일한 내용을 검증하고 있기 때문에
동일한 검증 내용은 Word에서만 진행하면 될 것 같습니다!
예시로 들어주신 Exception 이야기가 굉장히 와닿네요...! 👍
|
|
||
| @Test | ||
| void 모든_ResultType이_Matched라면_매칭_성공_여부를_true로_반환한다() { | ||
| final Result result = new Result(List.of(ResultType.MATCHED, ResultType.MATCHED)); |
There was a problem hiding this comment.
Result는 Word의 개수랑 같아야 한다고 생각하는데 어떻게 생각하세요?
저희 팀도 고민했던 부분이라 코멘트 남깁니당!! 🥹
There was a problem hiding this comment.
Result와 Word의 글자 수가 같아야 한다는 말씀이시죠?!
저희도 고민했었는데, 비즈니스 로직 상 Result는 Word에 의해서만 생성되기 때문에
따로 제약을 두지 않았습니다!
이 테스트케이스에서는 개수에 관계없이 모든 ResultType이 Matched인 경우를 테스트하는 것이라서
내부의 ResultType의 개수를 2개로 해도 괜찮다고 생각했던 것 같습니다!
실제 용례를 생각한다면 5개로 하는 것이 좋을 것 같긴 하네요! 👍
|
|
||
| import static org.junit.jupiter.api.Assertions.*; | ||
|
|
||
| public class ResultsTest { |
| // given when then | ||
| assertThrowsExactly(IllegalArgumentException.class, () -> new Word("abcdef")); |
There was a problem hiding this comment.
경계값 분석 라는 블랙박스 테스트가 있는데요.
지금 6자에 대한 테스트만 실행하고 있는데요.
5자 경계에 해당하는 주변 값들을 @ParameterizedTest와 함께 이용해서 테스트한다면
더 좋은 테스트가 될 수 있습니다.
링크 걸어두었으니 시간 되시면 함께 봐주세용!!
There was a problem hiding this comment.
너무 좋은 테스트 방법이네요! 참고하겠습니다. 감사합니다🩷
느낀점
처음 해보는 페어 프로그래밍이었는데, 생각했던 것보다 훨씬 재밌고 유익한 시간이었습니다.
다른 사람이 코드를 짜는 것을 눈 앞에서 지켜보고 제가 코드를 짜는 것도 누군가가 계속 지켜본다는 게 조금 무서웠지만, 막상 해보니 서로의 방식을 따라가보며 많은 것을 배울 수 있어 좋았습니다.
특히 우지님이 TDD나 좋은 객체지향 설계에 대해 많은 의견을 주셔서 더욱 많이 배우고 인사이트를 얻을 수 있었습니다!
기회가 된다면 업무에서도 페어 프로그래밍을 적용해보고 싶습니다.
잘한점
아쉬운점
배운점