[5기] 3주차 Wordle 과제 제출 - 우지 #32
Conversation
There was a problem hiding this comment.
우지님네 팀이 문서 정리도 잘해주시고, 코드도 너무 읽기 쉽게 잘 구성해주셔서 리뷰하는 시간 동안 즐겁게 리뷰를 할 수 있었습니다 👍 리뷰어의 입장을 고려한다면 이렇게 하는게 좋겠구나 라는 것을 많이 깨달을 수 있었네요 ㅋㅋ
코드를 보면서 정말 많은 것을 배웠지만, 그 중에서도 특히 검증 로직을 세밀하게 설계하신 부분이 가장 좋았던 것 같아요. 저는 구현에만 집착한 나머지 요구사항 외 검증을 크게 신경쓰지 못했거든요.😓
이 외에도 전체적으로 많은 것을 배워갑니다. 제 리뷰에서도 많은 것을 얻으실 수 있다면 너무 좋겠네요!
ps. 우지님과의 리뷰 핑퐁이 기대됩니다 😉
| public class ResultsTest { | ||
|
|
||
| @Test | ||
| void 결과들을_가지고_있다면_개수를_반환할_수_있다() { |
There was a problem hiding this comment.
테스트의 설명이 더 명확해지면 좋을 것 같습니다! 현재는 결과들이 없다면 개수를 반환하지 않을 것 같다는 느낌도 드네요😅
There was a problem hiding this comment.
음 이 부분에 대해서는 명확하지 않는 점이 있는 것 같군요 👍🏻
더 좋은 네이밍이 있는지 한번 더 고민해보겠습니다.
| public Result examine(final Guess guess) { | ||
| final List<ResultType> matchedResults = IntStream.range(START_INDEX, word.size()) | ||
| .mapToObj(index -> { | ||
| final Alphabet answerAlphabet = find(index); | ||
| final Alphabet guessAlphabet = guess.find(index); | ||
| if (answerAlphabet.equals(guessAlphabet)) { | ||
| return ResultType.MATCHED; | ||
| } | ||
| return ResultType.NONE; | ||
| }) | ||
| .toList(); | ||
|
|
||
| final Map<Alphabet, Long> unmatchedAnswerCounts = IntStream.range(START_INDEX, word.size()) | ||
| .filter(index -> ResultType.NONE.equals(matchedResults.get(index))) | ||
| .mapToObj(this::find) | ||
| .collect(Collectors.groupingBy(Function.identity(), Collectors.counting())); | ||
|
|
||
| final List<ResultType> resultTypes = IntStream.range(START_INDEX, matchedResults.size()) | ||
| .mapToObj(index -> { | ||
| if (ResultType.MATCHED.equals(matchedResults.get(index))) { | ||
| return ResultType.MATCHED; | ||
| } | ||
| final Alphabet alphabet = guess.find(index); | ||
| final long count = unmatchedAnswerCounts.getOrDefault(alphabet, DEFAULT_COUNT); | ||
| if (count > DEFAULT_COUNT) { | ||
| unmatchedAnswerCounts.put(alphabet, Math.subtractExact(count, DECREASE_COUNT_UNIT)); | ||
| return ResultType.EXIST; | ||
| } | ||
| return ResultType.MISMATCHED; | ||
| }) | ||
| .toList(); | ||
| return new Result(resultTypes); | ||
| } |
There was a problem hiding this comment.
우선 동작하는 코드로 작성은 완료했으나...
이걸 어떻게 깔끔하게 리팩토링하지라는 고민이 많네요 😢
There was a problem hiding this comment.
너무 고생하셨습니다 👍 잘 구동이 되고, 테스트케이스도 많이 작성 해주셔서 너무 좋았습니다!
저도 고통받았던 기억이 새록새록.. 😅 특히 노란색 타일(ResultType.EXIST) 채우는거 구현하다가 뇌가 녹는줄 알았네요 ㅋㅋㅋ
우지님 고민을 좀 덜어드려보고 싶어서, 코드를 최대한 해치지 않으면서 어떻게 하면 간결하게 만들어볼 수 있을까.. 하고 몇번 시도를 해봤는데, 제가 더 이상한 코드를 만들어내는 것 같네요 😓😓
제가 리팩토링을 시도하면서 느낀 점은, index가 발목을 잡고있는 것 같다는 생각이 들었어요. 제가 index를 사용하려고 시도했을 때 비슷한 코드가 만들어졌던 기억이 있거든요..!
혹시 index를 position형태로 갖는 GameAlphabet같은 객체를 만드는걸 시도 해보시는건 어떨까요? 사실 이건 제가 구현해본 방식이긴 한데, 좋은 방법일지, 혹은 우지님의 방향성과 맞는 방향일지는 모르겠어요 🤔
- 사실 알파벳이 position이라는 것을 갖는게 어색하기도 한 것 같고요.. ㅋㅋㅋ (저는 Letter 라는 객체를 썼는데, Letter에 position이 있으니 약간 어색한 상황이 오는 것 같더라고요 ㅎㅎ; )
src/main/java/wordle/Game.java
Outdated
| private Guess guess(final Dictionary dictionary) { | ||
| try { | ||
| outputView.insertWord(); | ||
| final String word = inputView.inputWord(); | ||
| if (dictionary.isExist(word)) { | ||
| return new Guess(word); | ||
| } | ||
| return guess(dictionary); | ||
| } catch (final Exception e) { | ||
| outputView.wrongWord(); | ||
| return guess(dictionary); | ||
| } | ||
| } |
There was a problem hiding this comment.
isExist()로 있는지 확인을 따로 진행하셨군요! 흐름이 더 자연스러워진 것 같아요 👍
그런데, 예외 관련 버그가 있어요..! 😢
5글자가 아닌 단어와 5글자이지만 사전에 없는 단어의 경우, if문에서 걸러지고 바로 guess()로 재귀를 타버려서 의도와는 다르게 예외가 발생하지 않네요..!
There was a problem hiding this comment.
헉 이런 버그가 있었다니, 정확히는 outputView.wrongWord(); 가 호출되지 않는 이슈군요
코드 중복보다는 아래와 같이 예외를 던지는 방향으로 개발해야겠습니다.
| private Guess guess(final Dictionary dictionary) { | |
| try { | |
| outputView.insertWord(); | |
| final String word = inputView.inputWord(); | |
| if (dictionary.isExist(word)) { | |
| return new Guess(word); | |
| } | |
| return guess(dictionary); | |
| } catch (final Exception e) { | |
| outputView.wrongWord(); | |
| return guess(dictionary); | |
| } | |
| } | |
| private Guess guess(final Dictionary dictionary) { | |
| try { | |
| outputView.insertWord(); | |
| final String word = inputView.inputWord(); | |
| if (dictionary.isExist(word)) { | |
| return new Guess(word); | |
| } | |
| throw new IllegalArgumentException(); | |
| } catch (final Exception e) { | |
| outputView.wrongWord(); | |
| return guess(dictionary); | |
| } | |
| } |
| public Result examine(final Guess guess) { | ||
| final List<ResultType> matchedResults = IntStream.range(START_INDEX, word.size()) | ||
| .mapToObj(index -> { | ||
| final Alphabet answerAlphabet = find(index); | ||
| final Alphabet guessAlphabet = guess.find(index); | ||
| if (answerAlphabet.equals(guessAlphabet)) { | ||
| return ResultType.MATCHED; | ||
| } | ||
| return ResultType.NONE; | ||
| }) | ||
| .toList(); | ||
|
|
||
| final Map<Alphabet, Long> unmatchedAnswerCounts = IntStream.range(START_INDEX, word.size()) | ||
| .filter(index -> ResultType.NONE.equals(matchedResults.get(index))) | ||
| .mapToObj(this::find) | ||
| .collect(Collectors.groupingBy(Function.identity(), Collectors.counting())); | ||
|
|
||
| final List<ResultType> resultTypes = IntStream.range(START_INDEX, matchedResults.size()) | ||
| .mapToObj(index -> { | ||
| if (ResultType.MATCHED.equals(matchedResults.get(index))) { | ||
| return ResultType.MATCHED; | ||
| } | ||
| final Alphabet alphabet = guess.find(index); | ||
| final long count = unmatchedAnswerCounts.getOrDefault(alphabet, DEFAULT_COUNT); | ||
| if (count > DEFAULT_COUNT) { | ||
| unmatchedAnswerCounts.put(alphabet, Math.subtractExact(count, DECREASE_COUNT_UNIT)); | ||
| return ResultType.EXIST; | ||
| } | ||
| return ResultType.MISMATCHED; | ||
| }) | ||
| .toList(); | ||
| return new Result(resultTypes); | ||
| } |
There was a problem hiding this comment.
너무 고생하셨습니다 👍 잘 구동이 되고, 테스트케이스도 많이 작성 해주셔서 너무 좋았습니다!
저도 고통받았던 기억이 새록새록.. 😅 특히 노란색 타일(ResultType.EXIST) 채우는거 구현하다가 뇌가 녹는줄 알았네요 ㅋㅋㅋ
우지님 고민을 좀 덜어드려보고 싶어서, 코드를 최대한 해치지 않으면서 어떻게 하면 간결하게 만들어볼 수 있을까.. 하고 몇번 시도를 해봤는데, 제가 더 이상한 코드를 만들어내는 것 같네요 😓😓
제가 리팩토링을 시도하면서 느낀 점은, index가 발목을 잡고있는 것 같다는 생각이 들었어요. 제가 index를 사용하려고 시도했을 때 비슷한 코드가 만들어졌던 기억이 있거든요..!
혹시 index를 position형태로 갖는 GameAlphabet같은 객체를 만드는걸 시도 해보시는건 어떨까요? 사실 이건 제가 구현해본 방식이긴 한데, 좋은 방법일지, 혹은 우지님의 방향성과 맞는 방향일지는 모르겠어요 🤔
- 사실 알파벳이 position이라는 것을 갖는게 어색하기도 한 것 같고요.. ㅋㅋㅋ (저는 Letter 라는 객체를 썼는데, Letter에 position이 있으니 약간 어색한 상황이 오는 것 같더라고요 ㅎㅎ; )
| public Word select(final List<? extends Word> wordList) { | ||
| final LocalDate now = LocalDate.now(); | ||
| final long nowEpochDay = now.toEpochDay(); | ||
| final long baseEpochDay = BASE_LOCAL_DATE.toEpochDay(); | ||
| final long timeDifference = Math.subtractExact(nowEpochDay, baseEpochDay); | ||
| if (timeDifference < 0) { | ||
| throw new RuntimeException("시간 차이가 음수로 나와서 단어를 선택할 수 없습니다"); | ||
| } | ||
| return wordList.get((int) timeDifference % wordList.size()); | ||
| } |
There was a problem hiding this comment.
해당 클래스의 select() 메서드는 비즈니스에서 꽤나 중요한 로직에 참여하고 있는데다가, 예외 상황까지 있어서 테스트가 작성되면 좋을 것 같다는 생각이 드는데요! 혹시 요 메서드를 테스트로 작성하는 것에 대해 어떻게 생각하실까요?
There was a problem hiding this comment.
음.. 사실 구현체에 대해서 테스트 생각을 해보지 않았었는데,
생각해보니, 우리가 실제로 사용할 클래스이고 중요한 요소이므로 테스트 하는게 맞을 것 같다는 생각이 드네요.
테스트를 어떻게 할지 고민했는데, LocalDate.now() 와 BASE_LOCAL_DATE 를 생성자로 받는 형태로 바꾸면 될 것 같네요 👍🏻
느낀점 및 배운점
진행에 앞서
왜 페어프로그래밍을 하는 것일까?라며 미션의 목표를 찾으려고 노력했습니다.페어프로그래밍 경험을 위해서?페어로부터 기술적인 배움을 얻기 위해서?페어에게 내 노하우를 공유하고 싶어서?여러 이유가 있겠지만, 제가 생각하는 이번 미션의 주된 목적은
팀원(페어)간의 커뮤니케이션이라고 생각합니다.우리는 개발자(기술자)로서 더 나은 코드를 작성하기 위해 생각하고 노력하는 것이 당연합니다.
하지만, 사람이 모두 다르듯이
더 좋은 코드를 작성하는 방법또한 다르기 마련입니다.페어 프로그래밍의 유용성에 대해서 많이 들었지만, 직접 경험해보는 것은 이번이 처음이었습니다.
따라서 이번 미션을 진행할 때, 처음에는
기술적인 퀄리티를 중요하는 것보다페어와의 커뮤니케이션에 집중을 했습니다.(그리고 아래에 적어 놓았지만, 커뮤니케이션이 잘 되면 자연스레
기술적인 퀄리티도 따라온다는 것을 느꼈습니다.)우선
커뮤니케이션을 잘하기 위해서는 서로가 생각하는 개발의 방향성을 맞추는 것이 좋다고 생각했습니다.3번의 회의에서
도메인 탐구,유스케이스 정의,UML(클래스/시퀀스) 다이어그램,테스트 케이스를 통해서 서로의 생각을 맞추었고이 과정에서 어떻게 객체를 설계하면 좋을지, 그리고 이를 어떻게 구현하면 좋을지 논의하면서 모델링에 대해서 배울 수 있었던 것 같습니다.
서로가 목표하는 것이 동일하니 이후 모데링을 코드로 옮기는데에 어려움은 크지 않았고
간혹, 구현하기 어려워 고민하고 있는 부분에 대해서 페어분과 바로 바로 소통하면서 빠르게 문제를 해결할 수 있었던 것 같습니다.
(다른 의견이 나올 때도, 건전한 토론을 통해서 더 나은 방향으로 쉽게 개발을 할 수 있었던 것 같습니다.)
주기적으로 커뮤니케이션 하면서 개발하고, 개선할 포인트들이 나오면 논의해서 수정하고
이런 과정을 반복하다보니
기술적인 퀄리티도 자연스레 따라온다는 것을 느꼈고페어프로그래밍에 대한 경험은 물론,페어프로그래밍의 여러 장점들을 느낄 수 있었습니다.ps. 같이 미션을 진행해준 최고의 페어 구월님께 너무나 감사드립니다.