-
Notifications
You must be signed in to change notification settings - Fork 709
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
base: nice7677
Are you sure you want to change the base?
3단계 - 사다리(게임 실행) #1806
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.
진우님, 3단계 미션 잘 구현하셨습니다. 👍
코멘트 조금 남겼습니다.
코멘트 확인 부탁드립니다. 🙂
|
||
private Ladder ladder; | ||
|
||
private Result result; |
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.
ladder
, result
를 필드로 관리할 필요가 있을까요?
run
메서드의 지역 변수로 관리해도 괜찮지 않을까요?
|
||
public static List<Boolean> generatorPoints(int count) { | ||
List<Boolean> points = new ArrayList<>(); | ||
IntStream.range(0, count) |
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.
IntStream
을 사용하신 이유가 있을까요?
for loop를 사용하는 것이 가독성이 더 좋을 것 같기도 합니다.
forEach
내부에서 generatorPoints의 지역변수인 points
에 접근하는 것은 외부 효과를 유발하는 것이므로 지양하는 것이 좋습니다.
public static boolean generator() { | ||
return random.nextBoolean(); | ||
|
||
public static List<Boolean> generatorPoints(int count) { |
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.
메서드 이름이니 generatePoints
와 같이 동사로 시작하는 이름을 사용하면 어떨까요?
|
||
private void printPlayerResult(String inputPlayer, List<Player> players) { | ||
|
||
ResultView.printResultText(); |
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.
사용자의 결과를 출력하기 위해서 printPlayerResultWithName
, printPlayerResult
를 호출하는데 이 메서드 안에서 "실행 결과"도 출력하도록 하면 어떨까요?
"실행 결과"를 출력하기 위해 printResultText
를 호출한 후 위의 메서드를 호출해야 한다는 것은 ResultView
의 상세 구현을 LadderGame
이 알고 순서에 맞춰 메서드를 호출해야 한다는 것으로 보입니다.
@@ -17,4 +19,38 @@ public String getName() { | |||
return name; | |||
} | |||
|
|||
public int getPlayerResultIndex(int currentPoint, Ladder ladder) { |
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.
사다리를 타고 내려가면서 최종 위치를 확인하는 것인 ladder
스스로 할 수 있지 않을까요?
ladder
클래스에 이 메서드를 구현해보면 어떨까요?
return points; | ||
} | ||
|
||
private static void addPoint(List<Boolean> points, int index, boolean point) { |
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.
포인트를 만드는 규칙은 Line
에서 관리하거나, Point
와 같은 클래스를 만들어서 관리해보면 어떨까요?
RandomUtil
인데 단순히 난수를 생성하는 것이 아니라, 라인을 만들기 위한 규칙에 대해서 너무 자세히 알고 있는 것으로 보이네요.
RandomUtil
은 generatorPoints
, addPoint
등의 도메인 기능은 구현하지 말고, 임의의 boolean 값을 생성하는 기능만 제공하는 형태로 구현해보시면 좋겠습니다.
} | ||
|
||
private void saveLadder(Ladder ladder) { | ||
this.ladder = ladder; | ||
} | ||
|
||
private void addLadderLines(int height, int width) { |
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.
Line
을 만드는 것을 외부에서 만들어서 Ladder
에 저장해주는데, Ladder
가 높이를 알고 있으니 Ladder
내부에서 필요한만큼 Line
을 만들어보면 어떨까요?
if (index == 0 || !points.get(index - 1)) { | ||
points.add(RandomUtil.generator()); | ||
return; | ||
public boolean hasLeftPoint(int currentPlayerPoint) { |
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.
각 점에서 우측으로 연결되는 선이 있는지 여부를 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(" ")); |
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.
사용자의 이름을 출력할 때 띄어쓰기가 미션의 실행 결과와 다른 것으로 보입니다.
미션의 실행 결과는 아래와 같습니다.
pobi honux crong jk
|-----| |-----|
| |-----| |
|-----| | |
| |-----| |
|-----| |-----|
꽝 5000 꽝 3000
구현하신 프로그램의 실행결과는 다음과 같습니다.
pobi honux crong jk
| |-----| |
|-----| | |
|-----| | |
| |-----| |
|-----| |-----|
꽝 5000 꽝 3000
미션의 실행 결과를 참고하시어 수정해보시면 좋겠습니다.
return; | ||
} | ||
|
||
int point = players.indexOf(new Player(inputPlayer)); |
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.
all
을 입력하지 않은 경우에는 추가로 결과를 보고 싶은 사람을 입력할 수 있는 것으로 보입니다.
실행 결과를 참고하시어 수정해보시면 좋겠습니다.
No description provided.