-
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
[3주차] Wordle 과제 제출 - 고슴도치파 #12
base: main
Are you sure you want to change the base?
Conversation
@@ -0,0 +1,7 @@ | |||
package wordle.domain.exception; | |||
|
|||
public class WrongUserWordException extends 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.
커스텀 예외 클래스를 사용하셨네요. 👍
|
||
public class GameManagerImpl implements GameManager { | ||
|
||
private static final int PROGRESS_COUNT = 6; |
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.
매직 넘버 상수 선언 👍
간단하게 설명드리면 design package 는 도메인을 정의할 때 사용한 인터페이스 모음입니다. 깃 숙련도가 부족하여 페어프로그래밍할 때 깃 커밋을 기능별로 쪼개지 못했고 조금 꼬여서 in, out 은 크게 도메인으로 어떤 데이터를 전달해주는 객체들을 의미했고 저는 답을 생성하는 부분도 in 이라 생각했습니다. 사용자의 입력일 수도 있고, 완전히 랜덤 데이터일 수도 있고 그 구현은 얼마든지 바뀔 수 있는 부분이므로 도메인에서는 인터페이스를, 구현은 in package 에 생성해두었습니다. |
for (int i = 0; i < input.length(); ++i) { | ||
if (inputUSed[i]) { | ||
continue; | ||
} | ||
for (int j = 0; j < ans.length(); ++j) { | ||
if (!ansUsed[j] && (input.charAt(i) == ans.charAt(j))) { | ||
ansUsed[j] = true; | ||
inputUSed[i] = true; | ||
judgeResult.setColor(i, Color.YELLOW); | ||
break; |
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.
프로그래밍 요구사항을 보면 indent(들여쓰기)를 2까지 허용
이라고 되어있습니다.
이중 포문 안에 if문을 선언하게 되면서 indent가 3이 되었네요. 😥
private MessagePrinter messagePrinter; | ||
private UserInput userInput; |
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.
이후에 변경될 일이 없다면 final을 붙여주는건 어떨까요? 🧐
@@ -0,0 +1,17 @@ | |||
package wordle.domain.vo; | |||
|
|||
public enum Color { |
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을 활용하여 책임을 확실하게 분리해주셨네요. 👍
@@ -0,0 +1,17 @@ | |||
package wordle.domain.vo; | |||
|
|||
public enum Message { |
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.
출력 메시지들에 대한 재사용성을 고려해주신 것 같아요.
재사용성을 위해서 메시지 전용으로 만들기 보다는 캡슐화가 되는 위치에 존재하도록 해서 응집도를 높여주는 건 어떻게 생각하시나요? 🧐
} | ||
|
||
public static UserWord of(String value) { | ||
validate(value); |
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.
입력 파라미터에 대한 검증 👍
|
||
private static void validate(String value) throws WrongUserWordException { | ||
if (value.length() != 5) { | ||
throw new WrongUserWordException("문자열의 길이가 잘못되었습니다!"); |
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.
커스텀 예외 클래스를 잘 활용해 주셨네요. 👍
추가로 예외가 왜 발생했는지에 대한 실패 정보를 같이 알려주면 어떨까요?
이펙티브 자바 Item 75. 예외의 상세 메시지에 실패 관련 정보를 담아라
@Override | ||
public String toString() { | ||
StringBuilder sb = new StringBuilder(); | ||
Arrays.stream(result).forEach(it -> sb.append(it.getBox())); | ||
return sb.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을 잘 활용해주셨네요. 👍
@@ -0,0 +1,7 @@ | |||
package wordle.domain.design; | |||
|
|||
public interface GameManager { |
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.
오~ interface를 만들어주셨네요. 👍
의존성 역전시키기 위함을 의도한 것이라고 이해했는데 제가 의도 파악을 맞게 한걸까요?
interface를 만들어 줄 생각은 하지 못했었는데 하나 배워갑니다!
import wordle.domain.vo.JudgeResult; | ||
import wordle.domain.vo.UserWord; | ||
|
||
class JudgementTest { |
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.
핵심 로직에 대한 테스트 작성을 해주시고 BDD 표준을 활용해주셔서 테스트를 잘 작성해주셨네요. 🙂👍👍
assertThat(colors[0]).isEqualTo(Color.GREEN); | ||
assertThat(colors[1]).isEqualTo(Color.GREEN); | ||
assertThat(colors[2]).isEqualTo(Color.GREEN); | ||
assertThat(colors[3]).isEqualTo(Color.GREEN); | ||
assertThat(colors[4]).isEqualTo(Color.GREEN); |
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.
여러 개의 Assertion 이 있을 경우 assertAll
을 사용해 보시는 건 어떨까요? 🧐
assertAll을 이용하게 되면 중간에 실패가 나더라도 이후 모든 케이스의 테스트를 진행하여 결과를 확인 할 수 있습니다.
import wordle.domain.vo.JudgeResult; | ||
import wordle.domain.vo.UserWord; | ||
|
||
class JudgementTest { |
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.
기능 요구사항
- 두 개의 동일한 문자를 입력하고 그중 하나가 회색으로 표시되면 해당 문자 중 하나만 최종 단어에 나타난다.
정답: acqde
입력: fqoqe
이런 경우에 GRAY, YELLOW, GRAY, GRAY, GREEN 으로 정답에는 q가 한 개이지만 입력한 문자열에 q가 중복된 경우에 대해 테스트 케이스를 추가해 더 다양한 테스트 케이스를 작성해보시는 건 어떠실까요? 🧐
return new UserWord(value); | ||
} | ||
|
||
private static void validate(String value) throws WrongUserWordException { |
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.
예외 처리에 대한 테스트 케이스를 추가해 보시는 건 어떠신가요?
@@ -0,0 +1,15 @@ | |||
## 기능분석 |
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.
요구 사항에 대한 구현을 정리해 주셨네요. 👍
No description provided.