-
Notifications
You must be signed in to change notification settings - Fork 43
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
[5기]3주차 Wordle 과제 제출 - kr #31
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
안녕하세요 kr님!! 늦은 리뷰드려서 죄송합니다.
미션 진행하시느라 고생하셨어요~~ 몇가지 저의 생각을 남겨보았는데 보시고 궁금하신 점이나 이야기 나누고싶은 부분은 코멘트 또는 DM주세요!!
감사합니다 😊
this.console = new Console(); | ||
this.answer = new Answer(); | ||
this.roundResults = new StringBuilder(); | ||
this.currentRound = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
매직넘버는 상수로 빼면 어떨까요~? 또는 Config로 게임관련된 설정을 관리하고 있는 것 같은데 요런 부분도 한곳에 모아두면 좋을 것같네요!
currentRound++; | ||
String input = console.userInput(); | ||
round = new Round(input); | ||
roundResults.append(round.roundResult(answer)).append("\n"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"\n"를 더해주는건 UI적 요소인거같은데 Console이 책임을 갖는게 더 좋지않을까 싶은데 어떻게 생각하시나요?🙂
public enum Tile { | ||
GREEN("🟩"), | ||
YELLOW("🟨"), | ||
GRAY("⬜️"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
게임 결과를 Enum으로 정의하셨네요~~ 다형성을 활용하는 것 너무 좋습니다.
하지만 위에서 말씀드린것처럼 UI와의 결합이 높아보여요!
if (InputValidator.isNotValid(input)) { | ||
throw new IllegalArgumentException(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
input에 대한 검증 책임을 InputValidator라는 유틸성 클래스에게 주고있는데, VO(값객체)를 생성하여 스스로 검증할 수 있도록 만들어주는건 어떨까요? 그러면 더욱 응집도가 높아질거같아요~ 그리고 IllegalArgumentException 을 공통적으로 던지는것 같은데 custom Exception을 통해 케이스를 좀 나눠보는건 어떨까요??
protected Answer(String value) { | ||
this.value = value; | ||
countPerCharacter(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
테스트코드에서만 사용되는 생성자이기때문에 protected로 정의한걸까용?
private final StringBuilder result; | ||
|
||
public RoundResult() { | ||
this.countPerTile = new EnumMap<>(Tile.class); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EnumMpa의 활용 너무 좋습니다~
@Override | ||
public String toString() { | ||
return result.toString(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
toString을 Tile과 RoundResult에 정의해주셨는데 이부분도 UI와의 결합이 높아보여요~ 😢
private static boolean isNotInWords(String input) { | ||
return !words.contains(input); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
메서드명에 부정어구를 넣는것 보다 긍정어구를 넣는게 가독성이 더 좋습니다!
https://latte1114.tistory.com/519
while (InputValidator.isNotValid(input)) { | ||
System.out.println("다시 입력해주세요."); | ||
input = sc.nextLine(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Console에서 input이 비지니스 규칙을 지키고 있는지 검증하는 것 같은데, 이를 Console에서 할 필요가 있을까요~?
표현계층에서는 데이터 포맷정도만 검증해도 괜찮을것같아요~~
void 입력_null() { | ||
Assertions.assertTrue(InputValidator.isNotValid(null)); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@NullAndEmptySource
의 사용법을 아시나요~~?
진행 방식
느낀점