-
Notifications
You must be signed in to change notification settings - Fork 24
2주차 미션 / 서버 1조 김종혁 #36
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
2주차 미션 너무 수고 많으셨습니다! 객체지향 원칙을 지키려고 많이 고민하신 것 같습니다. 사다리를 출력하는 작업을 LadderPainter 클래스로 분리하신 것, LadderCreator 인터페이스를 통해 ArtificialLadderCreator와 RandomLadderCreator 클래스를 구현한 것이 각각 단일 책임 원칙과 추상화가 잘 된 예시 같습니다. 테스트 코드도 꼼꼼히 작성해주셨네요! 더 생각해볼만한 코멘트 남겨두겠습니다! 참고해주세요 |
| public class LadderPainter { | ||
|
|
||
|
|
||
|
|
||
|
|
||
| public void printLadder(String prefix, LadderPosition ladderPosition, Row[] rows) { | ||
| System.out.println(prefix + ladderToString(ladderPosition,rows)); | ||
| } | ||
|
|
||
|
|
||
| public void printLadderStructure(Row[] rows){ | ||
| StringBuilder sb = new StringBuilder(); | ||
| for (Row row : rows) { | ||
| sb.append(row.nodeToString()); | ||
| sb.append("\n"); | ||
| } | ||
| System.out.println(sb); | ||
| } | ||
|
|
||
|
|
||
| private String ladderToString(LadderPosition ladderPosition, Row[] rows) { | ||
| StringBuilder sb = new StringBuilder(); | ||
| for (int i = 0; i < rows.length; i++) { | ||
| sb.append(rows[i].nodeToString(i, ladderPosition)); | ||
| sb.append("\n"); | ||
| } | ||
| return sb.toString(); | ||
| } | ||
|
|
||
|
|
||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
사다리 출력 작업을 LadderPainter에 분리하므로서 단일 책임 원칙을 더 잘 지킬 수 있을 것 같습니다! 다만 Row[]를 매번 메서드의 인자로 전달하는 것보단, 생성자로 주입 받는 것이 좋을 것 같아요. 현재 모든 메서드에서 rows를 사용 중이며 이를 매개변수로 전달하여 LadderPainter와 Row[] 객체의 결합도가 높아보입니다. 의존성 주입 방법 중 생성자 주입에 대해 알아보시면 좋을 것 같습니다!
public class LadderPainter {
Row[] rows;
public LadderPainter(Row[] rows) {
this.rows = rows;
}
public void printLadder(String prefix, LadderPosition ladderPosition,) {
....
}
public void printLadderStructure(){
....
}
private String ladderToString(LadderPosition ladderPosition) {
...
}
}| public int run(Position position) { | ||
|
|
||
|
|
||
|
|
||
| for (int i = 0; i < rows.length; i++) { | ||
| ladderPainter.printLadder("Before\n", new LadderPosition(Position.from(i), position), rows); | ||
| rows[i].nextPosition(position); | ||
| ladderPainter.printLadder("After\n", new LadderPosition(Position.from(i), position), rows); | ||
| } | ||
| return position.getValue(); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
사다리의 실행을 담당하는 메서드를 잘 작성해주셨습니다! 이동 전과 후를 명시하는 Before, After를 enum으로 표현하면 더 좋을 것 같습니다. LadderPainter 클래스의 내부 상황을 모르는 사람이 봤을 때, 매개변수 타입만으로 어떤 값을 전달해야 하는지 더 명확해질 것 같아요.
| public class LadderRunner { | ||
|
|
||
| private final Row[] rows; | ||
|
|
||
| LadderPainter ladderPainter = new LadderPainter(); | ||
|
|
||
| public LadderRunner(Row[] rows) { | ||
| this.rows = rows; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LadderPainter를 생성자를 통해 주입 받는 것이 어떨까요? LadderPainter에서 Row[]를 생성자에서 받은 후, 생성된 LadderPainter를 LadderRunner에 전달하면 LadderRunner, Row[], LadderPainter 세 클래스 간의 결합도를 낮출 수 있을 것 같아요.
| private final LadderCreator ladderCreator; | ||
|
|
||
| public LadderGame(ArtificialLadderCreator ladderCreator) { | ||
| this.ladderCreator = ladderCreator; | ||
| } | ||
| public LadderGame(RandomLadderCreator ladderCreator) { | ||
| this.ladderCreator = ladderCreator; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
현재 방법도 좋지만 생성자를 하나만 두고 LadderCreator로 지정하면 DIP 원칙에 더 부합할 것 같습니다!
private final LadderCreator ladderCreator;
public LadderGame(LadderCreator ladderCreator) {
this.ladderCreator = ladderCreator;
}
1week-completed에 이어서 구현했습니다
기존 LadderCreator을 ArtificailLadderCreator로 수정하고 RandomLadderCreator을 추가로 구현해 LadderCreator interface를 이용해 DI를 구현하고자 했습니다.
LadderPainter을 추가해 LadderRunner.run() 메서드를 실행 시 안에 printLadder 메서드를 호출해 게임 진행 과정 마다 사다리가 출력되도록 했습니다.
LadderPainter 내부의 메서드는 printLadder (게임을 진행할 때 LadderRunner 내부에서 호출하는 메서드), printLadderStructure(게임 진행 전 사다리의 구조를 출력하는 메서드), ladderToString(printLadder에서 호출됨, 각 행 별 String 구조를 모아서 String으로 반환) 세 종류가 있습니다.
Test 는 RandomLadderCreator Test 와 DrawLadderTest 두 종류를 구현했습니다.