2주차 미션 / 서버 1조 문승우 #38
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
sh1220
reviewed
Sep 28, 2024
Comment on lines
+12
to
+15
| public static ArrayIndex of(int index) { | ||
| validateIndex(index); | ||
| return new ArrayIndex(index); | ||
| } |
There was a problem hiding this comment.
정적 팩토리 메서드에서 보통 of는 받는 값이 2개 이상일때, from은 1개일때 자주 사용해요
이런 경우에는 from을 추천드려요!
Comment on lines
+39
to
+53
| private void vaildateLine(unsignedInt position, Direction direction) { | ||
|
|
||
| if(position.getValue() < 0 || position.getValue() >= LineLength()) { | ||
| throw new IllegalArgumentException(ExceptionMessage.INVALID_DRAW_POSITION.getMessage()); | ||
| } | ||
|
|
||
| int lineEndingAt = position.getValue() + direction.getValue(); | ||
| if(lineEndingAt < 0 || lineEndingAt >= LineLength()) { | ||
| throw new IllegalArgumentException(ExceptionMessage.INVALID_DRAW_POSITION.getMessage()); | ||
| } | ||
|
|
||
| if((nodes[position.getValue()].getState() != nodeState.empty) || (nodes[lineEndingAt].getState() != nodeState.empty)) { | ||
| throw new IllegalArgumentException(ExceptionMessage.INVALID_DRAW_POSITION.getMessage()); | ||
| } | ||
| } |
There was a problem hiding this comment.
if문 조건으로 있는 position 안의 값에 대한 검증, node 안의 값에 대한 검증 로직은 각 클래스에 있는게 더 좋아보여요!
Comment on lines
+40
to
+46
| private boolean invalidLine(Line line) { | ||
| if(isInvalidRow(line) || isInvalidColumn(line)){ | ||
| return true; | ||
| } else { | ||
| return false; | ||
| } | ||
| } |
There was a problem hiding this comment.
바로 이런 경우에도 조건문을 바로 리턴하면 깔끔할 것 같아요.
Suggested change
| private boolean invalidLine(Line line) { | |
| if(isInvalidRow(line) || isInvalidColumn(line)){ | |
| return true; | |
| } else { | |
| return false; | |
| } | |
| } | |
| private boolean invalidLine(Line line) { | |
| return isInvalidRow(line) || isInvalidColumn(line) | |
| } |
| } | ||
|
|
||
| private void validateLadder(unsignedInt row, unsignedInt column) { | ||
| if(row.getValue() <= 2 || column.getValue() <= 2){ |
There was a problem hiding this comment.
여기도 row, column 안의 값에 대한 것은 해당 클래스 안에서 검증하도록 책임을 분리하는 것이 좋을 것 같아요!
|
|
||
| validateLineInformation(line); | ||
|
|
||
| rows[line.getRow()].drawLineAt(unsignedInt.from(line.getColumn()), line.getDirection()); |
There was a problem hiding this comment.
line을 get으로 분해했다가, 다시 wrap하는 것이 디미터 법칙을 해쳐요.
이렇게 구조가 복잡해지는 경우에는, line을 받아서 그리는 메서드를 만드는 것도 좋아보여요.
Comment on lines
+6
to
+15
| public class LadderGameFactory { | ||
|
|
||
| public static LadderGame createPureLadderGame(int row, int column) { | ||
| return new LadderGame(pureLadderCreator.from(unsignedInt.from(row), unsignedInt.from(column))); | ||
| } | ||
|
|
||
| public static LadderGame createRandomLadderGame(int row, int column) { | ||
| return new LadderGame(randomLadderCreator.from(unsignedInt.from(row), unsignedInt.from(column))); | ||
| } | ||
| } |
There was a problem hiding this comment.
이런 경우에는 row, column을 unsignedInt로 받는 것이 더 좋아보여요.
Comment on lines
+16
to
+40
| private void drawRandomLines() { | ||
|
|
||
| int rowsLength = rows.length; | ||
| int colLength = rows[0].LineLength(); | ||
|
|
||
| int numberOfRandomLines = (int)Math.floor(rowsLength * colLength * 0.3); | ||
| boolean[][] overlapLine = new boolean[rowsLength][colLength]; | ||
| ArrayList<Line> lineList = new ArrayList<>(); | ||
|
|
||
|
|
||
| while(!isEnoughRandomLine(lineList, numberOfRandomLines)){ | ||
|
|
||
| int rowIndex = (int)Math.floor(Math.random()*rowsLength); | ||
| int colIndex = (int)Math.floor(Math.random()*(colLength-1)); | ||
|
|
||
| if(isLineValid(colIndex, overlapLine[rowIndex])){ | ||
| lineList.add(Line.of(rowIndex, colIndex, Direction.RIGHT)); | ||
|
|
||
| overlapLine[rowIndex][colIndex] = true; | ||
| overlapLine[rowIndex][colIndex+1] = true; | ||
| } | ||
| } | ||
|
|
||
| drawLinesInList(lineList); | ||
|
|
There was a problem hiding this comment.
바로 그리지 않고 굳이 overlapLine라인으로 따로 뺀 이유가 있을까요?
바로 그리는 편이 좋아보여요!
| import static org.assertj.core.api.Assertions.assertThat; | ||
| import static org.assertj.core.api.Assertions.assertThatThrownBy; | ||
|
|
||
| public class LadderGameTest { |
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.
테스트 작성 ( 이런 기능을 구현하겠다 ). => 구현 으로 하지 못하고
구현 => 구현에 맞는 테스트 작성의 방식으로 테스트 코드를 나중에 작성하게 되어 아쉬움이 남는 것 같습니다.
아직 TDD가 익숙하지 못했던 점도 있었고, 수업내용에 맞춰 이것저것 코드를 짜다 보니 구현을 끝까지 하게 되었던 것 같습니다.
객체지향적인 코드를 위해서 클래스로 분리, 클래스가 가진 책임을 분산시키는 것 또한
이번에 처음 고민하게 되었고, 최대한 한 객체 = 하나의 역활 에 집중해서 구현한 것 같습니다.
어떻게 하면 객체지향적으로 완성도 있는 코드를 효율적으로 작성할 수 있을지에 대해 계속 고민해 보려 합니다.
프로젝트를 시작할 때 이러한 역할을 하는 이러이러한 객체들을 만들겠다고 생각하고 시작하는 것도 하나의 방법인 것 같습니다.
여러모로 의미 있는 2주차였던 것 같습니다.
스터디에 참여해주신 부회장님과 파트장님들, 그리고 강의를 제공해주신 함형주 파트장님과 같이 스터디를 진행해주신 튜터님 그리고 부원분들에게 모두 감사합니다! 😊