Implement ladder game #7
Conversation
myangw
commented
Jan 31, 2020
myangw
commented
- Get user input (user names, height of ladder, result options)
- Draw ladder game
- Show results
- There was an issue that points between two ladder lines is printed as if they are connected. - To fix this issue, add fields to divide starting point and ending point in Point class
- Modify method from generating single random number to multiple random numbers
- add method to get result options input - seperate Input class from ladder game
- fixed test that was broken while modifying logic of generating single ladder point to multiple points per vertical line
- assure each move method of a row works - assure entire ladder game returns right result
- print entire results when user input is 'all'
Deocksoo
left a comment
Deocksoo
left a comment
There was a problem hiding this comment.
미향선생님 안녕하세요, 구현해주신 코드 확인했습니다 :)
코드 작성하시면서 고민하신 흔적이 많이 보이네요 ㅎㅎ 미션이 어려웠는데도 잘 구현해주신 것 같아요 :)
생각해보시면 좋을 만한 부분에 피드백 남겨두었으니, 리펙토링 하시면서 참고해주셔요~
|
|
||
| @Test | ||
| @DisplayName("사다리는 가로로 두번이상 연속 되지 않는다") | ||
| public void generateLadderPoint_notSubsequentLadder() { |
There was a problem hiding this comment.
해당 테스트 stackOverFlow 떨어지고 있어요, 확인 한 번 부탁드려요!
| } | ||
|
|
||
| private boolean twoLaddersContinuous(LadderPoint ladderPoint, List<LadderPoint> ladderPoints) { | ||
| return ladderPoints.contains(LadderPoint.of(ladderPoint.getX(), ladderPoint.getY() - 1)) && |
There was a problem hiding this comment.
이 부분의 로직이 잘 이해가 되지 않네요. 가로로 연속되어있는지 여부를 테스트할 때 Y값에 변화를 주어야 하는 이유가 무엇일까요..?
| assertThat(ladderStarter.generateRandomNum(4, Collections.singletonList(2))).isNotEqualTo(2); | ||
| } | ||
|
|
||
| public LadderGameStarter getGameStarterFixture() { |
|
|
||
| public void drawLadderLines(List<LadderPoint> ladderPoints) { | ||
| for (LadderPoint ladderPoint : ladderPoints) { | ||
| drawLadderLine(ladderPoint); |
There was a problem hiding this comment.
Row 에서 Line들을 직접 그려주고 있는데요,
layered archetecture 관점에서 보았을 때 domain 객체를 presentation layer와 분리해 주는 것이 낫지 않을까요?
| this.height = height; | ||
| } | ||
|
|
||
| public UserInput getUserInput() { |
There was a problem hiding this comment.
해당 메서드는 정적 펙터리 메서드로 만들어주면 어떨까요?
그러면 사용하지 않는 생성자나 불필요한 접근 제어자를 갖는 생성자도 자연스럽게 정리될 것 같아요 :)
| List<LadderPoint> ladderPoints = new ArrayList<>(); | ||
| ladderPoints.add(LadderPoint.of(1,1)); | ||
| ladderPoints.add(LadderPoint.of(3,1)); | ||
| row.drawLadderLines(ladderPoints); |
There was a problem hiding this comment.
move 메서드는 반드시 drawLadderLines가 선행되어야 하겠네요.
row가 draw되지 않고 사용되는 경우가 있나요? drawLadderLines 메서드를 수행한 적이 없는 ladder가 의미가 있는 ladder인가요?
처음에 row가 생성될 때, 사다리가 모두 연결되어있다는 것을 보장해주면 어떨까요?
3 participants
