[3주차] Wordle 과제 제출 - 하현 #19
Conversation
| List<GameHistory> gameHistories = new ArrayList<>(); | ||
|
|
||
| int count = 0; | ||
| while (count < InputView.GAME_TOTAL_ROUND && !answer.isSuccess()) { |
There was a problem hiding this comment.
while 조건문 내부를 더욱 파악하기 쉬운 메서드로 빼어내도 좋을 것 같아요~
There was a problem hiding this comment.
확실히 while문의 조건을 메서드로 빼내는게 좋을 것 같네요! 😊
|
|
||
| import java.util.Objects; | ||
|
|
||
| public class Letter { |
src/main/java/domain/Answer.java
Outdated
| private Tile getTile(Long count, Letter answerLetter, Letter letter) { | ||
| if (count == null || count <= 0) { | ||
| return Tile.GRAY; | ||
| } | ||
| if (answerLetter.equals(letter)) { | ||
| return Tile.GREEN; | ||
| } | ||
| return Tile.YELLOW; | ||
| } |
There was a problem hiding this comment.
Tile의 색상을 찾는 행위는 Tile 클래스 내부에서 처리해도 좋을 것 같아요~
There was a problem hiding this comment.
Tile의 색상을 찾는 행위는 Tile 클래스 내부에 위임하는게 올바른 역할인거 같네요 :)
src/main/java/domain/Word.java
Outdated
| if (this.isNotMatchWord(word)) { | ||
| throw new IllegalArgumentException(word + "는 5글자의 알파벳이 아닙니다."); | ||
| } |
There was a problem hiding this comment.
취향차이 일 것 같지만.. 유효성 검증 자체를 하나의 메서드에서 처리해도 좋을 것 같습니다!
There was a problem hiding this comment.
저는 유효성 검증을 메서드 별로 분리하는 편인데 유효성 검증이 하나인 경우에는 이런 식으로 처리하기도 하는 편입니다! 이번 미션에서는 최대한 피드백 반영 해보도록 할게요 😉
| import java.util.List; | ||
|
|
||
| public class WordsGenerator { | ||
|
|
There was a problem hiding this comment.
static 메서드만 존재해서 외부에서 객체 생성을 못하도록 막는건 어떨까요?
There was a problem hiding this comment.
확실히 해당 Util 클래스에는 외부 객체 생성을 못하도록 맞는게 맞는거 같습니다 다만 이런 경우에는 private로 접근제한자를 걸었는데 이런 경우엔 private로만 막아도 된다고 생각해서 걸었습니다. 혹시 이 부분에 대해서 어떻게 생각하는지 궁금합니다!
그리고 추가적으로 리플렉션을 사용하게되면 생성자를 막아도 생성을 할 수 있게 되는데 이런 경우엔 어떻게 생각하는지 의견을 듣고 싶습니다. 🤔
There was a problem hiding this comment.
�앗 넵, 저도 보통 접근제한자를 private으로 설정을 하는 편입니다!
말씀하신것처럼 리플렉션을 사용하면 private로 설정해도 접근이 가능해서 다른 방법이 없지 않을까 싶은데, 사실 이부분까지는 생각을 해본적이 없었습니다. ㅎㅎ;
접근을 완벽히 막는건 불가능하겠지만, 최소한의 방어 원칙(?)을 작성하는게 아닌가 싶습니다..
src/test/java/domain/AnswerTest.java
Outdated
|
|
||
| List<Tile> tiles1 = answer.compare(new Word("llsip")); | ||
| assertThat(tiles1).containsExactly(Tile.YELLOW, Tile.YELLOW, Tile.YELLOW, Tile.YELLOW, Tile.YELLOW); | ||
| // 실패발생 |
There was a problem hiding this comment.
페어프로그래밍 하면서 설명하던 주석이 남아있었네요 🥲
src/test/java/domain/AnswerTest.java
Outdated
| List<Tile> tiles = correctAnswer.compare(new Word("spill")); | ||
|
|
||
| assertThat(tiles).containsExactly(Tile.GREEN, Tile.GREEN, Tile.GREEN, Tile.GREEN, Tile.GREEN); | ||
| }*/ |
There was a problem hiding this comment.
혹시 위 코드들은 주석처리하고 삭제안하신 이유가 따로 있으실까요!?
There was a problem hiding this comment.
테스트 코드를 리팩토링하면서 지우지 못한 부분이 남아있었네요 😭
| assertThat(word.getWord()).isEqualTo(getLetters("SLiPP")); | ||
| } | ||
|
|
||
| @DisplayName("Word 가 5글자가 아니면 에러를 발생한다.") |
There was a problem hiding this comment.
5글자보다 많은 테스트 케이스도 존재하면 더욱 꼼꼼한 테스트가 될 것 같습니다!
There was a problem hiding this comment.
확실히 Word에 대한 다양한 케이스 테스트를 작성하지 않았는데 다양한 케이스를 추가해겠습니다
해당 부분을 테스트 메서드를 추가하는 것이 아닌 ParameterizedTest을 통해서 파라미터 케이스를 추가하는 방식으로 변경했는데 메서드를 아예 분리하는 것을 선호하실까요?
src/test/java/domain/WordTest.java
Outdated
| .isInstanceOf(IllegalArgumentException.class); | ||
| } | ||
|
|
||
| public List<Letter> getLetters(String word) { |
There was a problem hiding this comment.
테스트에서만 사용하는 것으로 보여서 private으로 제한해도 괜찮지 않을까요?
No description provided.