2주차 미션/ 서버 1조 정주연 #48
Conversation
kmw10693
left a comment
kmw10693
left a comment
There was a problem hiding this comment.
정말 고생하셨습니다! 코드에서 객체지향적으로 짜려는게 보여요! 테스트 코드도 다 구현하셔서 좋습니다!
|
|
||
| private final LadderCreator ladderCreator; | ||
| //private final LadderCreator ladderCreator; | ||
| private final LadderCreatorIF ladderCreator; |
| public class LadderPosition { | ||
| private int x, y; | ||
|
|
||
| public LadderPosition(int x, int y) { |
There was a problem hiding this comment.
정적 팩터리 of로 반환하니 private으로 만들어 무분별한 생성자 접근을 막는게 좋아보여요!
| public int run(LadderPosition position) { | ||
| for (int i = 0; i < rows.length; i++) { | ||
|
|
||
| printLadder(position, "BEFORE"); |
There was a problem hiding this comment.
BEFORE를 String으로 주는것보다, enum값을 추가하여 넣어주는게 나을것 같네요!
| rowStr.append(rows[y].getNodes()[x].toString().trim()).append(" "); | ||
| } | ||
| } | ||
| System.out.println(rowStr.toString().trim()); |
There was a problem hiding this comment.
System.out.print보다 logback을 사용하여 로그 형태로 출력하는게 나을것 같네요!
| private final GreaterThanOne numberOfRows; | ||
| private final GreaterThanOne numberOfPersons; | ||
|
|
||
| public LadderSize(int numberOfRows, int numberOfPersons) { |
There was a problem hiding this comment.
이 생성자도 private으로 만들고, from을 통해 초기화한 후 객체를 반환하는게 어떨까요?
| } | ||
| } | ||
| @Test | ||
| void 사다리_게임_진행_확인() { |
There was a problem hiding this comment.
사다리 게임 진행 확인 보다는 조금 더 구체적으로 쓰면 좋을것 같습니다. 예를 들어 사다리 게임에 처음에서 끝까지 도달하는지 확인 이런식으로요!
| LadderCreator ladderCreator = new LadderCreator(size); | ||
|
|
||
| //then | ||
| assertThat(ladderCreator).isNotNull(); |
There was a problem hiding this comment.
Isnotnull() 보다는 행과 열이 잘 생성되었는지 검증하면 어떨까요?
| LadderPosition position = LadderPosition.of(4, 0); | ||
|
|
||
| //then | ||
| assertThatThrownBy(() -> ladderGame.run(position)) |
There was a problem hiding this comment.
run 메서드를 실행했을때 어떤 예외클래스가 발생하고 , 예외 메시지가 발생했는지도 확인하면 좋을것 같네요!
|
|
||
| //then | ||
| assertThat(ladderGame.run(position)).isEqualTo(2); | ||
| ladderCreator.drawLine(LadderPosition.of(0, 0), LadderPosition.of(1, 0)); |
There was a problem hiding this comment.
drawline 메서드가 겹치니 private으로 빼서 공통 로직을 처리해보는게 어떨까요?
|
|
||
| //given | ||
| Position position = Position.from(0); | ||
| LadderPosition position = LadderPosition.of(0, 0); |
There was a problem hiding this comment.
생성자도 좋으나, 파라미터가 많을시에 빌더 패턴도 고려해보세요! 메서드 체이닝 방식으로 매개변수가 많을시 명확하게 객체를 생성 가능합니다!
2 participants
시간이 부족해 많이 미흡한 상태로 제출합니다..ㅜㅜ
현재 기능 구현은 완료 되었지만(?) , 테스트 시 자동사다리 생성 시 중복선 검증, 라인 유효성 검증이 완벽히 통과되지는 않는 상태입니다.
아직 리팩토링도 더 필요한 코드입니다.
리뷰해주시면 참고해서 수정하겠습니다!