2주차 미션 / 서버 2조 이찬양 #42
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
twkwon0417
reviewed
Sep 27, 2024
twkwon0417
left a comment
twkwon0417
left a comment
There was a problem hiding this comment.
꼼꼼히 잘 진행 하셨습니다. 💯
객체 지향적으로 짜시려 한게 느껴져서 코드 리뷰 하면서 저도 재밌었습니다 🥇
Comment on lines
+17
to
+19
| void drawLine(Position row, Position col); | ||
|
|
||
| void drawLine(); |
There was a problem hiding this comment.
row, col을 인자로 갖는 drawLine은 Default에서만 쓰이고 인자가 없는 drawLine은 Auto에서만 쓰이는데 abstract method로 선언힌 이유가 있을 까요?
| Position col = Position.from(randomInt % (ladderSize.getNumberOfPerson() - 1)); | ||
|
|
||
| try { | ||
| rows[row.getValue()].drawLine(col); |
There was a problem hiding this comment.
DefaultLadderCreator에서 drawLine 메서드와 같은 로직 인거 같아요! 재사용할수 있는 방법이 있지 않을까요?
There was a problem hiding this comment.
AutoLadderCreator가 사다리를 잘 생성했는지 unit testing하기 힘들어 보여요. 책임을 분리 하면 테스트를 더 쉽게 작성 할수 있을 것 같습니다!
Comment on lines
+10
to
+12
| public LadderGame(LadderCreator ladderCreator) { | ||
| this.ladderCreator = ladderCreator; | ||
| } |
| return position.getValue(); | ||
| } | ||
|
|
||
| private void printRows(int moveX, int moveY) { |
Comment on lines
+30
to
+40
| public void setRightNode() { | ||
| direction = RIGHT; | ||
| } | ||
|
|
||
| public void setLeftNode() { | ||
| direction = LEFT; | ||
| } | ||
|
|
||
| public boolean isAlreadySetDirection() { | ||
| return !isNone(); | ||
| } |
There was a problem hiding this comment.
Node의 값이 변경될때, 새로운 node를 생성하는 방식으로 처리해도 될거 같은데, setter을 사용한 특별한 이유가 있나요?
Comment on lines
+52
to
+68
| //given | ||
| Position position = Position.from(0); | ||
|
|
||
| //then | ||
| assertThat(ladderGame.run(position)).isEqualTo(2); | ||
|
|
||
| //given | ||
| position = Position.from(1); | ||
|
|
||
| //then | ||
| assertThat(ladderGame.run(position)).isEqualTo(1); | ||
|
|
||
| //given | ||
| position = Position.from(2); | ||
|
|
||
| //then | ||
| assertThat(ladderGame.run(position)).isEqualTo(0); |
There was a problem hiding this comment.
@ParmeterizedTest, @CsvSource 등 test관련 annotation에 대해 더 알아보면 좋을 거 같습니다 👍
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
✨사다리 출력, 사다리 자동 생성 기능 추가
♻️ 객체지향성 추가
코드리뷰 부탁합니다..! 🥹