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

3단계 - 사다리(게임 실행) #1806

Open
wants to merge 14 commits into
base: nice7677
Choose a base branch
from

Conversation

nice7677
Copy link

No description provided.

Copy link

@seokhongkim seokhongkim left a comment

Choose a reason for hiding this comment

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

진우님, 3단계 미션 잘 구현하셨습니다. 👍
코멘트 조금 남겼습니다.
코멘트 확인 부탁드립니다. 🙂


private Ladder ladder;

private Result result;

Choose a reason for hiding this comment

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

ladder, result를 필드로 관리할 필요가 있을까요?
run 메서드의 지역 변수로 관리해도 괜찮지 않을까요?


public static List<Boolean> generatorPoints(int count) {
List<Boolean> points = new ArrayList<>();
IntStream.range(0, count)

Choose a reason for hiding this comment

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

IntStream을 사용하신 이유가 있을까요?
for loop를 사용하는 것이 가독성이 더 좋을 것 같기도 합니다.
forEach 내부에서 generatorPoints의 지역변수인 points에 접근하는 것은 외부 효과를 유발하는 것이므로 지양하는 것이 좋습니다.

public static boolean generator() {
return random.nextBoolean();

public static List<Boolean> generatorPoints(int count) {

Choose a reason for hiding this comment

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

메서드 이름이니 generatePoints와 같이 동사로 시작하는 이름을 사용하면 어떨까요?


private void printPlayerResult(String inputPlayer, List<Player> players) {

ResultView.printResultText();

Choose a reason for hiding this comment

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

사용자의 결과를 출력하기 위해서 printPlayerResultWithName, printPlayerResult를 호출하는데 이 메서드 안에서 "실행 결과"도 출력하도록 하면 어떨까요?
"실행 결과"를 출력하기 위해 printResultText를 호출한 후 위의 메서드를 호출해야 한다는 것은 ResultView의 상세 구현을 LadderGame이 알고 순서에 맞춰 메서드를 호출해야 한다는 것으로 보입니다.

@@ -17,4 +19,38 @@ public String getName() {
return name;
}

public int getPlayerResultIndex(int currentPoint, Ladder ladder) {

Choose a reason for hiding this comment

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

사다리를 타고 내려가면서 최종 위치를 확인하는 것인 ladder 스스로 할 수 있지 않을까요?
ladder 클래스에 이 메서드를 구현해보면 어떨까요?

return points;
}

private static void addPoint(List<Boolean> points, int index, boolean point) {

Choose a reason for hiding this comment

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

포인트를 만드는 규칙은 Line에서 관리하거나, Point와 같은 클래스를 만들어서 관리해보면 어떨까요?
RandomUtil인데 단순히 난수를 생성하는 것이 아니라, 라인을 만들기 위한 규칙에 대해서 너무 자세히 알고 있는 것으로 보이네요.
RandomUtilgeneratorPoints, addPoint 등의 도메인 기능은 구현하지 말고, 임의의 boolean 값을 생성하는 기능만 제공하는 형태로 구현해보시면 좋겠습니다.

}

private void saveLadder(Ladder ladder) {
this.ladder = ladder;
}

private void addLadderLines(int height, int width) {

Choose a reason for hiding this comment

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

Line을 만드는 것을 외부에서 만들어서 Ladder에 저장해주는데, Ladder가 높이를 알고 있으니 Ladder 내부에서 필요한만큼 Line을 만들어보면 어떨까요?

if (index == 0 || !points.get(index - 1)) {
points.add(RandomUtil.generator());
return;
public boolean hasLeftPoint(int currentPlayerPoint) {

Choose a reason for hiding this comment

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

각 점에서 우측으로 연결되는 선이 있는지 여부를 boolean으로 관리하다보니 좌측 연결선이 있는지, 우측 연결선이 있는지를 인덱스를 이용해서 확인해야 하네요.
각 점을 좌측, 우측, 연결 없음을 나타내는 클래스로 표현해보면 어떨까요?

StringBuilder sb = new StringBuilder();
playerList.stream().map(Player::getName).forEach(name -> sb.append(String.format(NAME_LENGTH_FIVE_FORMAT_PATTERN, name)).append(" "));
players.stream().map(Player::getName).forEach(name -> sb.append(String.format(NAME_LENGTH_FIVE_FORMAT_PATTERN, name)).append(" "));

Choose a reason for hiding this comment

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

사용자의 이름을 출력할 때 띄어쓰기가 미션의 실행 결과와 다른 것으로 보입니다.

미션의 실행 결과는 아래와 같습니다.

pobi  honux crong   jk
    |-----|     |-----|
    |     |-----|     |
    |-----|     |     |
    |     |-----|     |
    |-----|     |-----|
꽝    5000  꽝    3000

구현하신 프로그램의 실행결과는 다음과 같습니다.

pobi  honux crong jk    
    |     |-----|     |
    |-----|     |     |
    |-----|     |     |
    |     |-----|     |
    |-----|     |-----|
꽝     5000  꽝     3000  

미션의 실행 결과를 참고하시어 수정해보시면 좋겠습니다.

https://edu.nextstep.camp/s/wLaV8qhA/ls/oHYSWjj7

return;
}

int point = players.indexOf(new Player(inputPlayer));

Choose a reason for hiding this comment

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

all을 입력하지 않은 경우에는 추가로 결과를 보고 싶은 사람을 입력할 수 있는 것으로 보입니다.
실행 결과를 참고하시어 수정해보시면 좋겠습니다.

https://edu.nextstep.camp/s/wLaV8qhA/ls/oHYSWjj7

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

Successfully merging this pull request may close these issues.

2 participants