Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
1 change: 1 addition & 0 deletions .idea/.name

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 12 additions & 0 deletions .idea/material_theme_project_new.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion .idea/misc.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

114 changes: 111 additions & 3 deletions src/main/java/Ladder.java
Original file line number Diff line number Diff line change
@@ -1,8 +1,116 @@
public class Ladder {
// Ladder
private final Matrix matrix; // should get Matrix's row and numberOfPerson
// private LadderLine LadderLine = new LadderLine();

private final int[][] rows;
public Ladder(Matrix matrix) {
// 사다리 생성하고 크기 지정해야 함
this.matrix = new Matrix(matrix.getRow(),matrix.getNumberOfPerson());
}
Copy link
Member

Choose a reason for hiding this comment

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

정적 팩토리 메서드를 통해 객체의 생성을 제어하고 있다면 생성자는 private 접근자로 제한해도 좋지 않을까요?


public static Ladder createLadder(NaturalNum row, NaturalNum numberOfPerson) {
Ladder ladder = new Ladder(new Matrix(row, numberOfPerson));
return new Ladder(new Matrix(row, numberOfPerson));
Copy link
Member

Choose a reason for hiding this comment

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

Ladder 의 생성 로직이 반복된건 실수겠죠? 👀

}

// Line is always drawn from left to right. (1을 먼저 대입하고 오른쪽에 -1 대입, 1 : 오른쪽이동 / -1 : 왼쪽이동)
// 연속된 라인은 허용하지 않는다

public void drawLine(LadderLine ladderLine) { // make LadderLine of ladder
// checkList
// If coordinate out of range(row / col(numberOfPerson)
// If that point is last col Line (only can draw to right)
// If that point has been drawn

int direction = 1;
Copy link
Member

Choose a reason for hiding this comment

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

1이라는 변하지 않는 내부 상수가 matrix.setDirection() 으로 전달되는 거라면 굳이 사용하지 않고 matrix 내부에서 제어하는 것이 좋지 않을까요?

// 이미 -1되이, 2차원 배열의 좌표 단위에 맞춰짐 (0부터 시작)
int x = ladderLine.getX();
int y = ladderLine.getY();

if (! checkCoordinateRange(x,y)) { // if coordinate out of range
throw new IllegalArgumentException("Out of coordinate range");
}

if (! checkColRange(x)) { // if that point is last col Line
throw new IllegalArgumentException("At the end of X coordinate can't draw to right");
}

if (! checkDrawn(x,y)) { // If that point has been drawn
throw new IllegalArgumentException("Point (x,y) has already been drawn");
}
Copy link
Member

Choose a reason for hiding this comment

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

검증 로직 잘 구현해 주셨군요! 검증 로직도 매우 중요하긴 하나 drawLine 이라는 메서드에 혼재되어 있으면 가독성이 떨어지게 됩니다. 검증 로직을 메서드로 추출해서 사용하시면 drawline 의 기능을 이해하기 좋을 것 같습니다!


// drawingLine
matrix.setDirection(x,y, direction);
matrix.printLadder();

}

// implement a movement along the line
public int run(NaturalNum line) { // choose num of ladder then can know where line to arrive
// check x coordinate is out of range

if (!checkCoordinateRange(line.getNaturalNum()))
throw new IllegalArgumentException("Out of starter line range");

return matrix.runLadder(line);
}

// -----------------------------------------------------------------------------------------------------
public int getRowsLength() {
return matrix.getRowsLength();
}

public Ladder(int row, int numberOfPerson) {
rows = new int[row][numberOfPerson];
public int getIntRow() {
return matrix.getIntRow();
}

public int getIntNumberOfPerson() {
return matrix.getIntNumberOfPerson();
}

public boolean checkCoordinateRange(int x, int y) {
if ((y > matrix.getIntRow()-1) && (x > matrix.getIntNumberOfPerson()-1))
return false;
Copy link
Member

Choose a reason for hiding this comment

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

if (조건식) retrun true; 같은 로직은 return 조건식 으로 사용하는게 깔끔할 것 같아요


return true;
}

public boolean checkColRange(int x) {
if (x > matrix.getIntNumberOfPerson() - 2)
return false;

return true;
}

public boolean checkDrawn(int x, int y) {
if (matrix.returnMatrixValue(x,y) == 1 || matrix.returnMatrixValue(x,y) == -1) {
return false;
Copy link
Member

Choose a reason for hiding this comment

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

Matrix로 클래스를 굳이 분리했는데 여기서 다시 값을 가져오는 건 객체 지향적이지 못하다는 생각이 듭니다. 해당 책임을 Matrix에 맡기고 메시지를 보내보는 건 어떨까요? 객체의 상태는 그 객체의 행동으로 관리할 수 있도록 노력해주세요!

Copy link
Author

Choose a reason for hiding this comment

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

이 메소드를 Matrix 클래스 내부에서 만들고, Ladder클래스에서는 Matrix로부터 성공과 실패 여부를 메시지로 받아오는 메소드로 만들라는 말씀이실까요??

}
return true;
}

// public int getMatrixValue(int x, int y) {
// return matrix.returnMatrixValue(x,y);
// }


// run메소드 starter line 검사
public boolean checkCoordinateRange(int x) {
if ((x > matrix.getIntNumberOfPerson()-1))
return false;

return true;
}
Copy link
Member

Choose a reason for hiding this comment

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

검증 메서드는 외부에서 사용하지 않는 것으로 보입니다. private 접근자로 제한하면 의도치 않은 사용을 막을 수 있어요!


}

// 객체지향 생활 체조 원칙 9단계
// 1. 메소드 하나에는 하나의 들여쓰기만
// 2. else 지양 (if문 안에서 early return)
// 3. 원시값과 문자열을 wrapper 객체로
// 4. method 체이닝 X
// 5. 축약X
// 6. Entity 적게 유지 (짧게하기)
// 7. 인스턴스는 1개만
// 8. 일급 컬렉션
// 9. getter / setter / property를 쓰지 않는다
30 changes: 30 additions & 0 deletions src/main/java/LadderLine.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Used when we drawLine, to control coordinate
// 일단 만들어야 할 거같아서 만듦

// Q : 좌표를 받아서 범위 유효성 검사를 이 클래스에서 진행하면 좋을 것 같은데 Ladder클래스의 멤버에 접근을...
public class LadderLine {
private int x, y;

private LadderLine(NaturalNum x) {
this.x = x.getNaturalNum() - 1;
}

private LadderLine(NaturalNum x, NaturalNum y) {
this.x = x.getNaturalNum() - 1;
this.y = y.getNaturalNum() - 1;
// x, y의 유효성 검사를 이 클래스에서 해야 할 거 같은데 row와 numberofPerson에 접근을 못해서 여기에 만듦

}

public static LadderLine from(NaturalNum x,NaturalNum y) {
return new LadderLine(x,y);
}

public int getX() {
return x;
}

public int getY() {
return y;
}
}
104 changes: 104 additions & 0 deletions src/main/java/Matrix.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
import java.io.IOException;

public class Matrix {
private final int[][] rows;
private final NaturalNum row, numberOfPerson; // y, x

// make ladder (row : height of ladder, col : numberOfPerson) (while checking parameter being naturalNum)
public Matrix(NaturalNum row, NaturalNum numberOfPerson) {
this.row = row;
this.numberOfPerson = numberOfPerson;
rows = new int[row.getNaturalNum()][numberOfPerson.getNaturalNum()];
}

public NaturalNum getRow() {
return row;
}

public NaturalNum getNumberOfPerson() {
return numberOfPerson;
}

public int getRowsLength() {
return rows.length * rows[0].length;
}

public int getIntRow() {
return row.getNaturalNum();
}

public int getIntNumberOfPerson() {
return numberOfPerson.getNaturalNum();
}

public void setDirection(int x, int y, int direction) {
rows[y][x] = direction;
rows[y][x + 1] = -direction;
}

public int returnMatrixValue(int x, int y) {
return rows[y][x]; // 이미 -1 되었음
}

public int runLadder(NaturalNum x) {
int starter = x.getNaturalNum() - 1;
Copy link
Member

Choose a reason for hiding this comment

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

굳이 NaturalNum으로 값을 받아서 -1을 해줘야 한다면 NaturalNum 말고 다른 검증 패턴을 가진 클래스가 필요하지 않을까요?

int currentRow = 0;
int finalLine = 0;


while (currentRow < row.getNaturalNum()) { // 높이 만큼 반복
Copy link
Member

Choose a reason for hiding this comment

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

fori 문으로 하면 가독성도 좋고 인덱스 값을 직관적으로 확인하기에도 좋을 것 같습니다.

// 양쪽 끝일 때는?
switch (rows[currentRow][starter]) {
case 0:
currentRow = moveDown(currentRow); // move to down
break;

case 1: // 일단 오른쪽으로 가면 다음은 무조건 내려가야 함
starter = moveRight(starter);
currentRow = moveDown(currentRow);
Copy link
Member

Choose a reason for hiding this comment

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

moveRight 메서드 이후에 moveDown 동작이 반드시 후행되어야 한다면 해당 로직을 moveRight에 포함시키면 좋을 것 같습니다.

break;

case -1: // 일단 왼쪽으로 가면 다음은 무조건 내려가야 함
starter = moveLeft(starter);
currentRow = moveDown(currentRow);
break;

default:
throw new IllegalArgumentException("There is unacceptable value in ladder");
Copy link
Member

@hamhyeongju hamhyeongju Sep 22, 2024

Choose a reason for hiding this comment

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

유효성 처리 이후 로직에 같은 예외를 발생하는 로직이 왜 필요할까요? 여기서 해당 예외가 발생하는 건 이미 유효성 검증에 실패한 코드라는 방증인 것 같습니다. 마치 유효성 검증을 끝낸 값을 디비에 저장하고 후에 조회할 때 다시 검증하는 것과 비슷해보입니다!

}
}

finalLine = starter + 1;
return finalLine; // 1부터 시작하므로
}

public int moveDown(int currentRow) {
return ++currentRow;
}

public int moveRight(int starter) {
if (starter >= numberOfPerson.getNaturalNum()-1) {
return starter;
}
return ++starter;
}

public int moveLeft(int starter) {
if (starter <= 0) {
return starter;
}
return --starter;
}

public void printLadder() {
for(int i = 0; i < row.getNaturalNum(); i++) {
for (int j = 0; j < numberOfPerson.getNaturalNum(); j++) {
System.out.print(rows[i][j] + " ");
}
System.out.println();
}
System.out.println("-".repeat(10));
}


}
33 changes: 33 additions & 0 deletions src/main/java/NaturalNum.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
public class NaturalNum {
private int naturalNum;

// static 메소드로 객체 생성하기 때문에, 생성자는 private으로 변경하였음
// Factory method와 private 생성자 (by study)
private NaturalNum() {
}

private NaturalNum(int naturalNum) {

if(checkNaturalNum(naturalNum)) {
this.naturalNum = naturalNum;
}

}

// Without making instance, can get naturalNum from int parameter
public static NaturalNum from(int i) {
return new NaturalNum(i);
}

public int getNaturalNum() {
return naturalNum;
}

public boolean checkNaturalNum(int naturalNum) {
if(naturalNum < 1) {
throw new IllegalArgumentException("Natural number must be positive");
}
return true;
}

}
40 changes: 40 additions & 0 deletions src/test/java/LadderTest.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,45 @@
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.*;
import static org.junit.jupiter.api.Assertions.*;

// All test should be done with Junit test code

// check Ladder class work normally
// check method of run return right value in various situation.
class LadderTest {
// 모든 테스트의 인수는 수학적 좌표 형식으로 주어야 함 !

@Test
@DisplayName("사다리 생성검사")
void createLadder() {
Ladder ladder = Ladder.createLadder(NaturalNum.from(3), NaturalNum.from(4));
// compare size of ladder with row * numberOfPerson
assertThat(ladder.getRowsLength()).isEqualTo(ladder.getIntRow() * ladder.getIntNumberOfPerson());
}

// Test if drawingLine acts normally and especially check how drawingLine acts at the right end.
@Test
void testDrawLine() {
Ladder ladder = Ladder.createLadder(NaturalNum.from(3), NaturalNum.from(4));
ladder.drawLine(LadderLine.from(NaturalNum.from(2), NaturalNum.from(2)));
ladder.drawLine(LadderLine.from(NaturalNum.from(3), NaturalNum.from(3)));
// assertThat(ladder.getMatrixValue(2,2)).isEqualTo(1);
// assertThat(ladder.getMatrixValue(3,2)).isEqualTo(-1);
}
//
// Test if it arrives right destination, when row is given
@Test
void testRunLadder() {
Ladder ladder = Ladder.createLadder(NaturalNum.from(3), NaturalNum.from(4));
ladder.drawLine(LadderLine.from(NaturalNum.from(2), NaturalNum.from(2)));
ladder.drawLine(LadderLine.from(NaturalNum.from(3), NaturalNum.from(3)));
assertThat(ladder.run(NaturalNum.from(2))).isEqualTo(4);
// assertThat(ladder.run(NaturalNum.from(1))).isEqualTo(4);
}

Copy link
Member

Choose a reason for hiding this comment

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

유효성 검증에 대한 테스트가 없는 것이 아쉽습니다! 유효성 검증은 어떻게 보면 핵심 비지니스 로직 만큼이나 중요하기 때문에 테스트에 포함되는 것이 바람직합니다! 검증에 대한 테스트 케이스가 없다면 리팩토링 이후에 해당 코드가 예외 상황을 잘 컨트롤한다고 확신할 수 있을까요?



}