2주차 미션 / 서버 3조 이윤희 #44
Open
Conversation
This file contains 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
JangIkhwan
reviewed
Sep 28, 2024
|
|
||
| LadderCreator ladderCreator; | ||
|
|
||
| public LadderGame(LadderCreator ladderCreator){ |
There was a problem hiding this comment.
인터페이스를 이용하니 아래와 같이 같은 기능을 하는 코드의 중복을 줄일 수 있었네요!
Comment on lines
+9
to
+15
| public static LadderGame createRandomLadderGame(LadderCreator randomCreator) { | ||
| return new LadderGame((LadderRandomCreator) randomCreator); | ||
| } | ||
|
|
||
| public static LadderGame createManualLadderGame(LadderCreator manualCreator) { | ||
| return new LadderGame((LadderManualCreator) manualCreator); | ||
| } |
There was a problem hiding this comment.
지금 팩토리는 Creator를 주입하는 것 외에 하는 일이 없네요. 팩토리가 game 생성에 필요한 작업을 모두 수행하는 게 좋을 것 같아요. 그러면 나중에 수정할 때 팩토리만 수정하면 되니 편하겠죠?
Comment on lines
+12
to
+15
| public void printRow(int rowNumber, int position, int currentRow) { | ||
| boolean isCurrent = (currentRow == rowNumber); | ||
| System.out.println(rows[rowNumber].rowToString(position, isCurrent)); | ||
| } |
There was a problem hiding this comment.
별을 찍을지 제어변수로 결정하는 아이디어 괜찮은 것 같아요. 그런데 이런 제어결합도는 혼란을 야기할 수 있으니 주의해서 사용하는 게 좋을 것 같습니다.
| } | ||
|
|
||
| public int getNumberOfRandomLine() { | ||
| return (int)Math.round(this.numberOfRow.getNumber() * this.numberOfPerson.getNumber() * 0.3); |
There was a problem hiding this comment.
int 형변환이나 round() 둘 중 하나만 써도 될 것 같네요!
| public StringBuilder rowToString(int position, boolean currentRow) { | ||
| StringBuilder rowString = new StringBuilder(); | ||
| for (int i = 0; i < nodes.length; i++) { | ||
| rowString.append(String.valueOf(nodes[i].getDirection().getValue())); |
There was a problem hiding this comment.
음 Node 객체 안에 Direction 객체가 있다는 걸 알아야해서 결합도가 늘어나는 것 같아 조금 아쉽네요. 노드의 값을 알아내는 역할을 Node에 맡겨 보면 어떨까요? Node는 메소드만 제공하는 거죠
Comment on lines
+41
to
+46
| boolean validLine = drawLine(ladderPos.getRow(), ladderPos.getColumn()); | ||
|
|
||
| if (validLine) { | ||
| lineCount.add(ladderPos); | ||
| } | ||
| } |
There was a problem hiding this comment.
drawLine의 반환값으로 사다리 가로줄을 다시 그어야할지 검증하네요! 괜찮아 보여요
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
No description provided.