Skip to content
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

숫자 야구게임 구현 #2

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Conversation

DevRunner21
Copy link
Collaborator

@DevRunner21 DevRunner21 commented Jan 13, 2022

  • readme 작성 후 개발진행
  • TDD로 개발 진행
  • 재귀용법 사용

재성님의 플이영상을 한번 봤더니
코드가 비슷해지는군요.. ㅜㅜ
자꾸 그게 생각이나다보니... ㅜㅜ
다음부터는 풀이강의는 최대한 늦게 봐야겠어요 ㅜㅜ

부족한 소스지만 시간 되실 때 리뷰 한번만 부탁드립니다!

제가 생각하기에 "좀 특이한 것 같다~" 라는 사항은 코멘트에 달아놨습니다.
감사합니다.

- 1~ 9까지의 숫자 체크
- 서로 다른 숫자 체크
    - 위치, 숫자 모두 맞춘경우 -> 스트라이크
    - 위치는 틀리고 숫자만 맞춘 경우 -> 볼
    - 위치, 숫자 모두 틀린 경우 -> 낫싱
Comment on lines +4 to +47

public abstract class GameInput {

public static final String REQUEST_RE_INPUT_MESSAGE = "다시 입력해주세요.";

public InputNumbers getUserInputNumbers() {

String inputStr = inputStr = input();
List<Integer> inputs = convertToIntegerList(inputStr);

InputNumbers inputNumbers = null;
try {
inputNumbers = new InputNumbers(inputs);

} catch (IllegalArgumentException e) {
System.out.println(REQUEST_RE_INPUT_MESSAGE);
getUserInputNumbers();
}

return inputNumbers;
}

public ConfirmCommand inputConfirmCommand() {
ConfirmCommand confirmCommand = null;

try {
confirmCommand = ConfirmCommand.findConfirmCommand(input());
} catch (IllegalArgumentException e) {
System.out.println(REQUEST_RE_INPUT_MESSAGE);
inputConfirmCommand();
}

return confirmCommand;
}

private List<Integer> convertToIntegerList(String inputStr) {
return Arrays.stream(inputStr.split(""))
.map(Integer::parseInt)
.collect(Collectors.toList());
}

protected abstract String input();

}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

입력 UI를 구현할 때, TemplateMethodPattern을 적용해봤습니다.

Comment on lines +39 to +46
ConfirmCommand confirmCommand = input.inputConfirmCommand();
if (confirmCommand.isExit()) {
return;
}

start(); // 재귀사용
return;
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

재원님이 말씀해주셨던 피드백중에 "재귀식"에 대한 내용이 기억나서
이번에 반복되는 처리에 대해서 재귀식으로 구성해봤습니다.

Comment on lines +3 to +19
public class RandomNumberGenerator {

private final int MAX = 9;

private final int MIN = 1;

private final Random random;

public RandomNumberGenerator() {
this.random = new Random();
}

public int generate() {
return random.nextInt((MAX - MIN) + 1) + MIN;
}

}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RandomNumber에 대한 생성의 책임은 RandomNumberGenerator라는 객체에게 부여했습니다.

this.number = number;
}

public int getPosition() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getter가 특별한 이유 없이 public으로 노출되고 있어요 🤔

private final List<Ball> balls;

public Balls(InputNumbers inputNumbers) {
balls = new ArrayList<>();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

생성자에서는 단순한 값의 초기화만 이루어져야 좋다고 생각해요.
생성자에서 단순히 List을 받고, static 팩토리 메서드로 다양한 생성 방법을 노출하면 어떨까요? 🤔

numbers.forEach(this::addInputNumber);
}

// TODO : getter가 있어도 될까?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getter로 노출 하는건 굉장히 좋지 못하다고 생각해요!

this.ballCount = ballCount;
}

public int getStrikeCount() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

strikeCount를 알려주지 않고 Score에게 strikeCount값을 물어보면 어떨까요?

}

public String makeResultMessage() {
StringBuilder stringBuilder = new StringBuilder();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ResultMessage를 만들어 주는건 도메인의 역할이 아니라고 생각해요

@@ -0,0 +1,19 @@
import java.util.Random;

public class RandomNumberGenerator {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

추상화 할 수 있을것 같아요🤔

@darkant99
Copy link
Member

고생 하셨습니다.
충분히 깔끔한 코드 이지만 아쉬운 점은 객체들간의 역할이 모호한것 같아요😭
또한 정말 특별한 이유가 아니라면 getter의 노출은 지양 되었으면 해요.
미션을 진행 하시면서 getter를 1개도 사용하지 않는다는 극단적인 방법을 사용 해보셔도 인사이트를 얻으실것 같아요 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants