Skip to content
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

[사다리 - 3단계] 김성환 미션 제출합니다. #24

Open
wants to merge 2 commits into
base: fanngineer
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/main/java/controller/LadderController.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ public Ladder createLadder() {
}

public void printResult(Ladder ladder){
outputView.printResult();
outputView.printResultText();
outputView.printLadder(ladder);
outputView.printResult(ladder.getParticipants());
}
}
39 changes: 39 additions & 0 deletions src/main/java/domain/Ladder.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,21 @@
import java.util.List;

public class Ladder {
private final List<Participant> participants;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ladder에서는 Participant를 직접 의존하는 것보다 사다리를 타는 과정에서만 협력을 하는 방안은 어떻게 생각하시나요?
혹시 Participant를 직접 의존하신 이유는 무엇인가요?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저는 일반적으로 사다리게임이 1회성이라 생각해서 하나의 사다리게임 안에 참여자들이 속해있다고 생각해서 그처럼 구현했습니다. 그런데 말씀 듣고나니 고정된 생각을 벗어나서 의존성을 줄이는 방법으로 구현하는게 좋을 것 같아 반영하겠습니다!

private final List<Line> lines;

public Ladder(Size ladderSize, Size lineSize) {
this.participants = generateParticipants(lineSize);
this.lines = generateLines(ladderSize, lineSize);
getResult();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

해당 메소드는 객체 생성 시 동작하는 것보다는 생성 후에 동작을 하는 것이 좋을 것 같습니다!
저는 개인적으로 생성자에서는 객체 생성의 역할까지만 수행해야한다고 생각합니다!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저도 분리하는 방향을 생각하다가 사다리게임 시작시에 결과가 정해진다고 생각해서 그렇게 했는데, 위와 같은 맥락으로 분리하는게 좋을 것 같네요!

}

private List<Participant> generateParticipants(Size lineSize) {
List<Participant> participants = new ArrayList<>();
for (int i = 0; i < lineSize.getSize(); i++) {
participants.add(new Participant(i));
}
return participants;
}

private List<Line> generateLines(Size ladderSize, Size lineSize) {
Expand All @@ -18,7 +29,35 @@ private List<Line> generateLines(Size ladderSize, Size lineSize) {
return lines;
}

private void getResult(){
for (Line line : lines) {
changeByLine(line);
}
}

private void changeByLine(Line line){
for(int i = 0; i< line.getPoints().size() ; i++){
changeByPoint(i,line.getPoints().get(i));
}
}

private void changeByPoint(int idx, Point point){
if(point.isConnected()){
swapEnds(idx);
}
}

private void swapEnds(int idx){
int temp = participants.get(idx).getEnd();
participants.get(idx).setEnd(participants.get(idx+1).getEnd());
participants.get(idx+1).setEnd(temp);
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

랜덤으로 생성된는 사다리에서 해당 메소드가 잘 동작하는지를 직접 돌려가면서 확인해가는 비용보다는 테스트 코드를 작성해서 확인하는 것이 보다 효율적일 것 같습니다. 해당 부분에 대한 테스트 작성해주세요


public List<Line> getLines() {
return lines;
}

public List<Participant> getParticipants() {
return participants;
}
}
23 changes: 23 additions & 0 deletions src/main/java/domain/Participant.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package domain;

public class Participant {
private final int start;
private int end;

public Participant(int start) {
this.start = start;
this.end = start;
}

public int getStart() {
return start;
}

public int getEnd() {
return end;
}

public void setEnd(int end) {
this.end = end;
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 메소드는 결국 setter의 기능이기는 하지만 move 혹은 이동의 의미를 나타내는 명칭을 이용하는 것이 코드를 읽은 과정에서 가독성이 좋을 것 같습니다!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

좋은 의견 감사드립니다😄

}
9 changes: 8 additions & 1 deletion src/main/java/view/OutputView.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@

import domain.Ladder;
import domain.Line;
import domain.Participant;
import domain.Point;
import java.util.List;

public class OutputView {

public void printResult(){
public void printResultText(){
System.out.println("실행결과");
}

Expand All @@ -25,4 +27,9 @@ public void printLine(Line line){
System.out.print("|");
}

public void printResult(List<Participant> participants){
for(Participant participant : participants){
System.out.println(participant.getStart() + " -> " + participant.getEnd());
}
}
}