2주차 미션 / 서버 4조 조동현 #31
Conversation
| LadderGame randomLadderGame = new LadderGame(randomLadderCreator); | ||
| randomLadderCreator.drawRandomLines(); | ||
| randomLadderGame.run(Position.from(0)); | ||
| System.out.println("왜이러지.."); |
There was a problem hiding this comment.
System.out.println("왜이러지..") <--- 이거 지우는거 깜박했네요!ㅋㅋㅋㅋ 잘 돌아갑니다
hamhyeongju
left a comment
hamhyeongju
left a comment
There was a problem hiding this comment.
코드 잘 봤습니다!
전반적으로 요구사항을 잘 이해하고 깔끔하게 작성해 주셔서 코드리뷰가 되게 편했네요! 막막할 수 있는 로직을 최대한 간단하게 생각하고 풀어내신게 인상깊었습니다! 객체 지향의 핵심인 객체에 적절한 책임을 부여하고 책임에 맞는 일을 하는 메서드 작성하는 것을 잘 이해하신 것 같아 뿌듯합니다!(강의를 통해 객체 지향을 잘 이해하신게 맞겠죠?ㅎㅎ)
미션 힌트에서 언급드렸던 조합에 대한 부분이 빠진게 거의 유일하게 아쉬웠네요. 코드 리뷰와 2week-completed 코드 보시면서 본인의 코드와 비교해보는 시간을 가져보시면 좋을 것 같습니다. 수고 많으셨습니다!
|
|
||
| import ladder.creator.LadderCreator; | ||
|
|
||
|
|
There was a problem hiding this comment.
아무 의미 없는 부분이 commit에 포함되어 있네요.. git add -A 하신거 딱 걸렸습니다 ㅎㅎ
인텔리제이 내장 git gui(이름을 이렇게 부르는게 맞는지 모르겠네요)에서 수정 전후 코드를 확인할 수 있으니 의미 없는 부분은 제거하고 commit 하시면 좋을 것 같습니다!
| private void printLadder(LadderPosition currentPosition){ | ||
| for (int i = 0; i < rows.length; i++) { | ||
| if (currentPosition.getRow().getValue() == i) { | ||
| System.out.println(rows[i].toString(currentPosition)); |
There was a problem hiding this comment.
LadderPosition을 row 값과 비교하는 로직은 Row에 있으면 더 좋을 것 같습니다!
| public String toString(LadderPosition currentPosition){ | ||
| StringBuilder sb = new StringBuilder(); | ||
| for (int i = 0; i < nodes.length; i++) { | ||
| sb.append(nodes[i].getDirection().getValue()); |
There was a problem hiding this comment.
Node에서 getDirection을 통해 값을 가져오는게 아니라 Node에서 직접 자신의 상태를 sb에 추가하는 방식이 더 객체 지향적일 것 같습니다. getter를 사용하기 전에 해당 클래스 안에서 처리할 수 있는 로직인지 고민해 보시는게 좋겠습니다.
| setDirectionBetweenNextPosition(startPosition); | ||
| } | ||
|
|
||
| public String toString(LadderPosition currentPosition){ |
There was a problem hiding this comment.
toString 이라는 이름은 최상위 클래스인 Object에 정의된 메서드와 이름이 같아 의미가 혼동될 수 있습니다. 다른 메서드 이름을 사용하는게 바람직합니다.
| sb.append(nodes[i].getDirection().getValue()); | ||
| if (i == currentPosition.getCol().getValue()){ | ||
| sb.append("*"); | ||
| } |
There was a problem hiding this comment.
이중 들여쓰기가 된 부분은 메서드 추출을 통해 분리해 보시는게 어떨까요?
| private final Row[] rows; | ||
| private int numberOfLines; | ||
| private int numberOfPerson; | ||
|
|
There was a problem hiding this comment.
미션 힌트에서 조합을 이용한 RandomLadderCreator를 구현을 언급했었는데, 만약 그랬다면 DefaultLadderCreator에서 정의했던 drawLine을 재사용할 수 있지 않았을까요?
| } | ||
|
|
||
| public void drawRandomLines(){ | ||
| Set<LadderPosition> drawnLines = new HashSet<LadderPosition>(); |
There was a problem hiding this comment.
객체를 기반으로 Set 등의 컬렉션을 다룰 때는 해당 객체에서 equals, hashCode를 구현해주어야 실질적으로 동일한 객체인지 구별할 수 있게 됩니다.
2 participants
출력과 랜덤미션 구현 완료! -> main 함수 돌리면 됩니다.
LadderCreator를 인터페이스로 만들어서 기능들을 묶고, defaultLadderCreator랑 randomLadderCreator로 implements받아서 구현
의도랑 잘 맞겠죠..? 맞기를!
모두들 화이팅 할만합니다