[1단계 - 장기] 헤일러(민서현) 미션 제출합니다. #10

threepebbles · 2025-03-20T07:50:44Z

안녕하세요! 반가워요 로키

이번에 장기 미션을 진행하면서 몇몇 새로운 시도들을 해보게 되었는데요.

장기라는 게임 자체가 낯설다보니 말의 이동 방식에 따라 말들의 공통된 기능을 묶어내는 것이 굉장히 어려웠던 것 같습니다.

코드가 길어 읽기 힘드시겠지만 리뷰 잘 부탁드립니다 😄

체크 리스트

미션의 필수 요구사항을 모두 구현했나요?
Gradle test를 실행했을 때, 모든 테스트가 정상적으로 통과했나요?
애플리케이션이 정상적으로 실행되나요?
헤일러의 프롤로그에 셀프 체크를 작성했나요?

객체지향 생활체조 요구사항을 얼마나 잘 충족했다고 생각하시나요?

1~5점 중에서 선택해주세요.

선택한 점수의 이유를 적어주세요.

장기판을 출력하는 로직과, 장기판을 초기화하는 로직에서 행과 열을 순회하기 위해 2중 for문을 사용하는 것이 불가피했습니다.
포의 움직임을 구현하는 로직에서 한 메서드당 10줄이라는 제한을 지키기 어려웠습니다.
Board와 JanggiManager의 역할과 책임을 뚜렷하게 구분하지 못했습니다.

어떤 부분에 집중하여 리뷰해야 할까요?

📌 노드와 엣지를 이용한 인접 리스트 구현 방식

장기판을 구현하기에 앞서서, 페어 프로그래밍 첫날은 장기판의 좌표를 어떻게 관리할 지에 대해 고민하며 코드에 거의 손도 대지 않았습니다.

현재 스텝에서는 궁과 궁성에 대한 것을 고려하지 않지만, 각 기물들의 움직임 패턴에 따라 장기판이 어떻게 바뀌어도 동작할 수 있게 만들고 싶었습니다.

그렇게 고민 끝에 노드와 엣지를 이용하여 그래프를 그리게 되었습니다.

제가 생각한 인접 리스트의 장점과 단점은 다음과 같습니다.

장점
- 현재 코드에서 “왕”이 위치한 곳의 노드에 대각선 방향의 단방향 엣지 16개(한의 궁성쪽 8개, 초의 궁성쪽 8개)만 추가해주면 다른 코드의 수정 하나 없이 궁성 영역에 대한 모든 기물들의 움직임이 룰대로 동작하게 될 것이라 예상하고 있습니다.
- 격자 밖을 벗어나는 것에 대한 검사가 따로 필요 없습니다. (노드 간의 이동은 엣지로 연결된 경우만 가능하기 때문)
단점
- 노드와 엣지를 처음에 초기화하는 과정이 필요합니다. (BoardGenerator.initializeNodesAndEdges())
- 노드와 엣지로 구성된 인접 리스트가 먼저 만들어지지 않으면 기물들의 움직임을 테스트할 수 없습니다. (기물들의 움직임이 노드에 의존적이게 됨)

기물들의 움직임이 노드 엣지에 종속적이게 된다는 단점이 있지만, 노드 엣지로 구성된 그래프만 완성되면 보드판의 변경과 확장에 더 유연하다 생각해서 적용해보았습니다. 적절하게 잘 사용했는지 궁금합니다!

📌 기물들의 움직임 패턴과 관련된 상수 관리

각 기물들의 움직임 패턴을 CHO_BYEONG_MOVABLE_DIRECTIONS, PIECE_PATHS과 같이 클래스 상단에 static final 상수로 관리했는데, 클래스 윗부분에 하드코딩된 부분이 많아서 가독성을 저하할 수 있겠다고 느껴졌습니다. 이런 부분들을 조금 개선하고 싶은데 어떻게 개선해야 할 지 방법이 잘 떠오르지 않습니다..!

public class Byeong implements Piece {

    private static final List<Direction> CHO_BYEONG_MOVABLE_DIRECTIONS
            = List.of(LEFT, UP_LEFT, UP, UP_RIGHT, RIGHT);
    private static final List<Direction> HAN_BYEONG_MOVABLE_DIRECTIONS
            = List.of(LEFT, DOWN_LEFT, DOWN, DOWN_RIGHT, RIGHT);
    private static final Map<Team, List<Direction>> DIRECTIONS_BY_TEAM = Map.ofEntries(
            Map.entry(Team.CHO, CHO_BYEONG_MOVABLE_DIRECTIONS),
            Map.entry(Team.HAN, HAN_BYEONG_MOVABLE_DIRECTIONS)
    );
    ...

public class Ma implements Piece {

    private static final List<PiecePath> PIECE_PATHS = List.of(
            new PiecePath(
                    List.of(new Directions(List.of(UP))), new Directions(List.of(UP, UP, LEFT))
            ),
            new PiecePath(
                    List.of(new Directions(List.of(UP))), new Directions(List.of(UP, UP, RIGHT))
            ),
            new PiecePath(
                    List.of(new Directions(List.of(RIGHT))), new Directions(List.of(RIGHT, RIGHT, UP))
            ),
            new PiecePath(
                    List.of(new Directions(List.of(RIGHT))), new Directions(List.of(RIGHT, RIGHT, DOWN))
            ),
            new PiecePath(
                    List.of(new Directions(List.of(DOWN))), new Directions(List.of(DOWN, DOWN, RIGHT))
            ),
            new PiecePath(
                    List.of(new Directions(List.of(DOWN))), new Directions(List.of(DOWN, DOWN, LEFT))
            ),
            new PiecePath(
                    List.of(new Directions(List.of(LEFT))), new Directions(List.of(LEFT, LEFT, DOWN))
            ),
            new PiecePath(
                    List.of(new Directions(List.of(LEFT))), new Directions(List.of(LEFT, LEFT, UP))
            )
    );
    ...

…) 추가

threepebbles · 2025-03-23T18:59:23Z

안녕하세요 로키!
피드백 주신 내용들 반영해보았습니다 😄
프롤로그 내용도 추가했습니다! 헤일러의 프롤로그
코멘트에 대한 답변과 함께 피드백 반영 및 리팩터링하면서 궁금했던 점 몇 가지 질문 드리려고 합니다!
잘 부탁드립니다 😁

💬 코멘트에 대한 답변

그래프 구조에서 나오는 용어들이라 비슷한 방식으로 쓰신게 아닐까 추측되기는하지만... 어떤 의도로 사용하고 계신건지 제가 잘 이해하지 못하겠네요. 🥲

Node와 Point가 언뜻보기에 비슷해보이는데 어떤 차이가 있는걸까요?

(제가 Node, Point, Edge에 대한 이해가 부족한 탓이겠지만) Board가 Map<Node, Piece> 만을 상태로 가져도 될 것 같은데, Map<Point, Node> 도 상태로 가지고있어서 더더욱 이해가 안되는 것 같아요. 😭

제가 처음 작성한 코드가 조금 난해하기도 하고, Node를 Board 밖으로 꺼내서 여기저기 남용해서 이해하기 어려우셨을 것 같습니다 😭
지금은 PointNodeMapper 클래스를 통해 모든 Node에 대한 정보를 Board안에 캡슐화해서 Board의 외부에서는 Node를 전혀 모르도록 리팩터링 했습니다. 아마도 처음 코드보다는 읽기 편하실 거라 생각이 들어요...!

인접 리스트 방식을 사용한 이유 (노드, 엣지)

Point(포인트)

public record Point(int row, int column) {
}

Point는 단순히 장기판에서의 좌표(row, column)만을 의미합니다. 좌표 그 이상의 역할을 하지 않습니다.

Node(노드)

public class Node {
    private final List<Edge> edges;
}

Node는 Point를 그래프의 개념으로 추상화한 객체입니다. Point는 그래프에서 Node와 동일한 개념이므로 Point와 Node는 각 좌표마다 일대일로 대응됩니다.
Node는 주변에 다른 Node와의 연결 정보(Edge)를 갖고 있다는 점이 Point와 다릅니다. 장기 판의 격자에서 연결되어 있는 선들은 전부 Edge에 해당합니다. Edge에는 방향이 존재하기 때문에 Node와 Node간 연결 정보를 단방향 Edge 2개로 나눠서 표시할 수 있습니다.

Edge(엣지)

public class Edge {
    private final Node nextNode;
    private final Direction direction;

    public Edge(final Node nextNode, final Direction direction) {
        this.nextNode = nextNode;
        this.direction = direction;
    }
}

Edge는 도착 지점의 노드 정보(nextNode)와, 방향 정보(Direction)를 갖고 있습니다.
Edge에 대한 자세한 설명은 그림과 코드 예제로 함께 하겠습니다.

@Test
void 간선이_있는_방향은_true_없는_방향은_false를_반환한다() {
    // given
    final Node upNode = new Node();
    final Node upRightNode = new Node();
    final Node centerNode = new Node();
    final Node rightNode = new Node();

    final Edge rightEdgeForUpNode = new Edge(upRightNode, RIGHT);
    final Edge downEdgeForUpNode = new Edge(centerNode, DOWN);
    upNode.addAllEdges(List.of(rightEdgeForUpNode, downEdgeForUpNode));

    final Edge leftEdgeForUpRightNode = new Edge(upNode, LEFT);
    final Edge downEdgeForUpRightNode = new Edge(rightNode, DOWN);
    upRightNode.addAllEdges(List.of(leftEdgeForUpRightNode, downEdgeForUpRightNode));

    final Edge upEdgeForCenterNode = new Edge(centerNode, UP);
    final Edge rightEdgeForCenterNode = new Edge(rightNode, RIGHT);
    centerNode.addAllEdges(List.of(upEdgeForCenterNode, rightEdgeForCenterNode));

    final Edge upEdgeForRightNode = new Edge(upRightNode, UP);
    final Edge leftEdgeForRightNode = new Edge(centerNode, LEFT);
    rightNode.addAllEdges(List.of(upEdgeForRightNode, leftEdgeForRightNode));

    // when & then
    SoftAssertions.assertSoftly(softly -> {
        softly.assertThat(upNode.hasNextNode(UP)).isFalse();
        softly.assertThat(upNode.hasNextNode(RIGHT)).isTrue();
        softly.assertThat(upNode.hasNextNode(DOWN)).isTrue();
        softly.assertThat(upNode.hasNextNode(LEFT)).isFalse();

        softly.assertThat(upRightNode.hasNextNode(UP)).isFalse();
        softly.assertThat(upRightNode.hasNextNode(RIGHT)).isFalse();
        softly.assertThat(upRightNode.hasNextNode(DOWN)).isTrue();
        softly.assertThat(upRightNode.hasNextNode(LEFT)).isTrue();

        softly.assertThat(centerNode.hasNextNode(UP)).isTrue();
        softly.assertThat(centerNode.hasNextNode(RIGHT)).isTrue();
        softly.assertThat(centerNode.hasNextNode(DOWN)).isFalse();
        softly.assertThat(centerNode.hasNextNode(LEFT)).isFalse();

        softly.assertThat(rightNode.hasNextNode(UP)).isTrue();
        softly.assertThat(rightNode.hasNextNode(RIGHT)).isFalse();
        softly.assertThat(rightNode.hasNextNode(DOWN)).isFalse();
        softly.assertThat(rightNode.hasNextNode(LEFT)).isTrue();

        softly.assertThat(centerNode.hasNextNode(UP_RIGHT)).isFalse(); // 중요
    });
}

왕, 사, 병, 차, 포와 같이 방향 기반으로 움직이는 기물들은 노드와 노드 사이가 엣지로 연결된 경로로만 이동 가능합니다.
이런 인접 리스트 방식은 그래프는 이 방향 기반으로 움직이는 기물들을 움직일 때 빛을 발합니다.

제가 맘대로 위와 같은 커스텀 장기판을 만들었다고 가정하겠습니다.
장기 룰에 의하면 위의 장기판 모양에서는 "차" 기물이 (2,2)에 있다면 (3,3), (4,4) (5,5)로는 이동 가능하고 (1,1), (6,6)로는 이동 불가능해야 합니다.

노드 엣지 기반으로 구현한 경우, 아래와 같은 코드로 "(5,5) 노드"까지만 이동 가능하다는 것을 쉽게 알 수 있습니다.

Node currentNode = "(2,2) 노드"; // pseudo code
Direction direction = DOWN_RIGHT;
List<Node> candidates = new ArrayList<>();  // 이동 가능한 노드 후보
while(currentNode.hasNextNode(direction)) {
    currentNode = currentNode.getNextNodeByDirection(direction);
    candidates.add(currentNode);
}

"(5,5) 노드".hasNextNode(DOWN_RIGHT)은 false이기 때문에 이동 가능 후보군(candidates)에 들어갈 수 없습니다.
"(1, 1) 노드".hasNextNode(UP_LEFT)도 false이므로 마찬가지입니다.

장기판 격자의 그림이 어떻게 바뀌어도 방향 기반의 기물들은 본인이 움직일 수 있는 방향대로 움직이고 싶은 만큼 움직일 수 있습니다. 즉, 장기판의 대각선 경로가 궁성이 아닌 다른 어디에 생겨도 병, 차, 왕, 사, 포는 본인의 움직임 패턴대로 움직일 수 있다는 장점이 있습니다. 이런 이유로 인접 리스트 표현 방식을 채용해보았습니다.

혹시 설명이 이해되셨을까요! 😄

❓ 여쭤보고 싶은 점

1️⃣ "상"과 "마"의 움직임 관리

public record Movement(List<Path> obstaclePaths, Path destinationPath) {
}

"도착점과 도착점에 가기 위해 확인해야 하는 장애물들의 위치는 관련이 깊기 때문에 묶어서 관리해야 한다."라는 관점에서 Movement라는 클래스로 List<Path> obstaclePaths (장애물들의 위치)와 Path destinationPath (도착점 위치)를 묶어서 관리하고 있습니다. Ma 클래스와 Sang 클래스 맨 위에 static final 상수로 두기에는 가독성을 많이 해치는 것 같아서 JumpingMovements 클래스에서 마상의 가능한 경로 목록을 enum 상수로 관리하도록 분리했습니다. JumpingMovements 클래스의 네이밍은 적절한지, 역할이 너무 없어서 그냥 Ma 클래스나 Sang 클래스 안에 상수로 정의해서 사용하는 것이 더 괜찮을 지 잘 모르겠습니다..!

2️⃣ 계층에 따라 일관되게 예외 처리하는 것이 괜찮을까요?

예외를 처리하는 방법은 복구, 회피, 전환(의도된 회피), 무시 이렇게 4가지 방법이 있다고 배웠습니다.

복구: 실패하면 될 때까지 재시도하는 방법
회피: 실패했을 때 예외를 발생시켜 외부로 넘기는 방법
전환: 외부에서 처리하되 추상화 레벨에 맞게 실패한 이유를 변경하는 방법 (ex. 내부적으로 발생한 예외를 catch해서 적절한 예외 메시지와 함께 새로운 Exception을 던짐)
무시: 실패했을 때 내부에서 조치하지 않고 외부에서도 문제가 발생했다는 사실을 알 수 없게 catch하면서 아무것도 안 하는 방법

지금까지 도메인 내의 로직에서 예외가 발생하는 경우 전환 방식을 채용하고,
실행의 흐름을 담당하는 클래스(현 JanggiGameManager)에서 예외를 잡아 복구하는 방식을 채용했습니다.
계층에 따라 일괄적으로 예외 처리하는 방식을 통일하는 게 괜찮은 방식일까요?

3️⃣ 전체적으로 방어적 검증(?)이 과하지는 않은지 피드백 받고 싶습니다.

class Board {
    ...
    public Point getNextPoint(final Point point, final Direction direction) {
        validateExistPoint(point);
        Node node = pointNodeMapper.getNodeByPoint(point);
        Node nextNode = node.getNextNodeByDirection(direction);
        return pointNodeMapper.getPointByNode(nextNode);
    }
}

위에서 getNextPoint()는 파라미터로 받은 point에서 direction 방향으로 이동할 수 있다면 direction 방향의 point를 반환하는 기능입니다. 외부에서 getNextPoint()를 사용할 때는 항상 board.existsNextPoint(point, direction)을 통해 이동 가능 여부를 항상 먼저 확인하도록 코드가 작성되어 있는데, getNextPoint() 내부에서 validateExistPoint(point);를 통해 검증을 한번 더 하도록 하는 게 괜찮은 방법일까요? 중복은 아닐까요? 검증의 책임을 잘 모아서 관리할 수 있는 방법이 있을까요?

private List<Point> findMovablePoints(final Point point, final Board board) {
    return DIRECTIONS_BY_TEAM.get(this.team).stream()
            .filter(direction -> board.existsNextPoint(point, direction)) // 이미 검사함
            .map(direction -> board.getNextPoint(point, direction))
            .filter(nextPoint -> !(board.existsPiece(nextPoint) && board.matchTeam(nextPoint, this.team)))
            .toList();
}

4️⃣ 생성자에 넣기에는 코드의 양이 너무 많을 때 어떻게 하시나요?

PointNodeMapper 기본 생성자에서 정석 장기판의 모양에 맞게 Point와 Node의 연관관계를 매핑해주기엔 코드의 양이 너무 많아서 PointNodeMapperFactory 클래스를 따로 분리했습니다... 😭
사실 팩터리 패턴을 쓰기에는 어울리지 않다고 생각을 하는데 다른 방법이 떠오르지 않았습니다.
로키는 기본 생성자에 넣기에는 너무 많은 셋업을 위한 코드가 필요한 경우 어떤 방식을 사용하시나요?

threepebbles · 2025-03-24T01:27:33Z

안녕하세요 로키
리뷰 요청 후에 네이밍 개선, 메서드 분리를 통해 가독성이 높아질 것 같은 부분이 있어서 추가 커밋 몇개 push했어요...!
버그도 하나 발견해서 고쳤습니다. 혹시 읽고 계셨다면 죄송합니다. 😭
이후로 추가 커밋 더 이상 하지 않겠습니다...!

Rok93

헤일러 피드백 반영 잘해주셨습니다. 👍💯
친절하고 꼼꼼하게 구현 의도 설명해주셔서 조금 복잡해보였던 로직이 하나씩 이해되기 시작했어요. 🤩
질문에 답변을 드리다보니 코멘트가 좀 많아졌는데, 1단계 충분히 잘 구현해주신 것 같아서,
2단계 진행하면서 같이 수정해주셔도 좋을 것 같아서 1단계는 이만 머지하도록하겠습니다. 😃

질문에 대한 답변과 추가 코멘트 남겨두었으니 확인 부탁드릴게요. 😉

Rok93 · 2025-03-24T13:46:29Z

src/main/java/domain/board/Board.java

+        return !isTwoWangsAlive();
+    }
+
+    private boolean isTwoWangsAlive() {


잘 생각해보면 isEnd의 반대니까 isPlaying() 같은 네이밍을 지어도 되지 않을까요?
두개의 왕이 살아있다는 네이밍 보다는 진행중이다가 조금 더 의도를 직관적으로 이해할 수 있지 않을까 싶어요. 😃

Rok93 · 2025-03-24T13:47:27Z

src/main/java/domain/board/Board.java

+
+    private Set<Team> findTeamsOfWang() {
+        return pieceByPoint.values().stream()
+                .filter(piece -> piece.type() == PieceType.WANG)


piece 객체에게 왕인지 여부를 메시지를 보내서 물어보면 어떨까요? 😃

현재 piece 객체에게 메시지를 보내서 물어보는 방법은 아래의 3가지가 떠오릅니다.
1번 방법: boolean isWang(), boolean isPo()를 추가한다.

piece.isWang();

2번 방법: boolean hasType(PieceType pieceType)을 추가한다.

piece.hasType(PieceType.WANG);

3번 방법: 현행 유지

step2.1에서 기물의 종류에 따라 점수를 부여해야 하기 때문에, 모든 기물들의 구체 클래스 타입을 확인해야 하므로 enum PieceType을 사용하지 않기는 어려울 것 같습니다. PieceType type() 기능이 있는데 이를 이용하지 않고 인터페이스에 boolean isWang()을 추가해서 사용하는 것은 인터페이스에 중복된 기능이 추가되는 느낌을 받았습니다. 로키는 어떻게 생각하시나요?

다시 생각해보니 추상 메서드로 int score()를 둔다면 PieceType을 사용하지 않아도 각 기물의 점수를 다형성을 이용해서 계산할 수 있겠네요..! 좀 더 고민해보겠습니다.

Rok93 · 2025-03-24T13:48:51Z

src/main/java/domain/board/Board.java

+    }
+
+    private boolean isTwoWangsAlive() {
+        return findTeamsOfWang().containsAll(List.of(Team.CHO, Team.HAN));


추가적으로 워딩을 생각했을 때, 아래와 같은 로직이 네이밍이랑 조금 더 매칭이 잘되지 않을까요? 🤔

Suggested change

return findTeamsOfWang().containsAll(List.of(Team.CHO, Team.HAN));

findTeamsOfWang().size() == 2;

장기의 원래 룰대로라면 각 팀의 왕은 1개씩만 존재해야 하지만, 아래와 같은 예외 상황이 발생할 수 있다고 생각해서 팀의 종류까지 확인하게 했어요!

한 팀의 왕이 2개 이상이 존재할 수 있는 경우

Rok93 · 2025-03-24T14:06:23Z

src/main/java/domain/board/Node.java

+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Objects;


사용하지 않는 import문인 것 같아요. 👀

Rok93 · 2025-03-24T14:12:53Z

src/main/java/domain/board/Board.java

+        return piece.canMove(source, destination, this);
+    }
+
+    public void movePiece(final Point source, final Point destination) {


이동할 수 있는 경로인 것 같은데 적의 말이 있으니 이동하지 못하는 것 같아요. 👀
적군의 말을 잡아먹지는 못하는 것 같은데, 이 부분 확인해주실 수 있을까요? 😃

테스트 해보았는데 move와 6,2 사이에 공백이 2개 입력되어서 해당 예외가 발생한 것 같아요!
커맨드와 좌표 사이에 여러 공백이 들어와도 하나로 처리할 수 있는 방법을 생각해보겠습니다. 🤔

Rok93 · 2025-03-24T15:03:53Z

src/main/java/domain/board/Board.java

+            return false;
+        }
+        Piece piece = getPieceByPoint(point);
+        return piece.type() == PieceType.PO;


piece 객체에게 PO와 타입이 같은지 메시지를 보내볼 수 있을 것 같아요.

Rok93 · 2025-03-24T15:41:20Z

src/main/java/domain/board/PointNodeMapper.java

+import java.util.Map;
+import java.util.stream.Collectors;
+
+public class PointNodeMapper {


이 객체가 필요한 이유도 제가 잘 이해했는지 모르겠는데요. 😭 😭
결국 이 객체는 특정 A 포인트에서 B 포인트로 가는 경로를 찾는데 이용이 되는 걸까요?
(PointNodeMapper 라는 네이밍보다는 조금 더 역할을 드러낼 수 있는 네이밍을 사용해보면 좋을 것 같아요, ex. 경로를 찾는 역할을 한다면.... PathFinder)

A -> x1 -> x2 -> x3... -> B의 경로를 찾을 수 있으니 장기말이 이동할 때, 지나가는 경로에 말이 있다면 이동하지 못하는 등의 판단을 하기위함의 용도가 맞는걸까요? 👀

nodeByPoint 와 pointByNode는 단순히 key, value를 뒤바꾼 케이스인 것 같은데요.
pointByNode가 필요한 이유가 있나요?

Rok93 · 2025-03-24T15:57:47Z

src/main/java/domain/piece/Po.java

+        return findHurdle(nextPoint, direction, board);
+    }
+
+    private void findCandidates(final Point currentPoint, final Direction direction, final Board board,


이 부분은 재귀함수를 활용하셨군요? 👍😉
알고리즘 고수의 향기가 물씬나는 헤일러도 이미 잘 아시겠지만 재귀함수는 가독성이 좋다는 장점이 있는 반면에
StackOverFlow 에러가 발생할 위험성이 있어요. 🙄
가능하면 재귀함수 대신에 다른 방법을 고민해보면 어떨까 싶어요.

Rok93 · 2025-03-24T16:02:12Z

src/main/java/domain/board/PointNodeMapperFactory.java

+        return new PointNodeMapper(nodeByPoint);
+    }
+
+    private void createAllNodes(Map<Point, Node> nodeByPoint) {


제가 이해한대로라면 PointNodeMapper는 몇번을 연산하든 똑같은 결과의 PointNodeMapper가 나올 것 같은데, 차라리 캐싱을 해두는 것도 방법일 것 같아요. 🤔

Rok93 · 2025-03-24T16:19:55Z

src/main/java/domain/board/JumpingMovements.java

+
+public enum JumpingMovements {
+
+    MA(


(질문) 상"과 "마"의 움직임 관리
"도착점과 도착점에 가기 위해 확인해야 하는 장애물들의 위치는 관련이 깊기 때문에 묶어서 관리해야 한다."라는 관점에서 Movement라는 클래스로 List obstaclePaths (장애물들의 위치)와 Path destinationPath (도착점 위치)를 묶어서 관리하고 있습니다. Ma 클래스와 Sang 클래스 맨 위에 static final 상수로 두기에는 가독성을 많이 해치는 것 같아서 JumpingMovements 클래스에서 마상의 가능한 경로 목록을 enum 상수로 관리하도록 분리했습니다. JumpingMovements 클래스의 네이밍은 적절한지, 역할이 너무 없어서 그냥 Ma 클래스나 Sang 클래스 안에 상수로 정의해서 사용하는 것이 더 괜찮을 지 잘 모르겠습니다..!

먼저 JumpingMovements 라는 네이밍의 의도는 무엇인지 궁금합니다. 👀
점핑의 의미가 조금 헷갈리는 것 같아요. 👀 (점핑의 의미가 말을 건너뛰는 의미로 쓰신건가했는데, 상이나 마는 그런 말이 아니었던거 같아서 조금 헷갈리네요. 🙂)

일단 저는 일관성있게 움직임을 정의하면 좋겠다는 생각이 먼저 들기는 합니다. 🤔
저는 현재 마, 상만 JumpingMovements를 활용해서 findMovablePoints를 수행하고있는데,
다른 피스들도 일관성있게 JumpingMovements를 활용해서 findMovablePoints를 하도록 해보면 어떨까 싶어요. 🤔

이렇게 일관성있게 코드를 변경한다면 JumpingMovements가 공통적으로 쓰이는 위치를 확인해보면서
적절히 위임할만한 역할이 없는지 살펴보면 좋을 것 같아요. 😃

혹시 제안드린 방식이 너무 어려운 변경이라면 제안드린 방식은 무시해주셔도 될 것 같아요. 😃

ljhee92 and others added 30 commits March 18, 2025 17:43

docs: 프로그램 실행 흐름 기능 작성

c05b6eb

feat(Point): 좌표 클래스 추가

bc91c23

feat(Direction): 방향을 의미하는 상수 추가

88f09bf

feat(Node, Edge): 노드, 엣지 클래스 추가

fedaaa3

feat(BoardInitializer): 노드와 엣지 정보 초기화

63c152f

feat(SangMaOrderCommand): 상마 순서를 의미하는 enum 추가

5448de9

feat: 팀을 구분하기 위한 enum 추가

b7eaff1

feat(Piece): 기물 역할 클래스 추가

627e724

feat(BoardInitializer): 초나라 기물 배치 초기화 기능 추가

cd7657f

feat(BoardInitializer): 상마 순서에 따른 기물 배치 초기화 기능 추가

1978af8

refactor: BoardInitializer 리팩터링

2ab255f

refactor: 패키지 이동

5ea034d

feat: 특정 좌표에 기물 존재 여부 확인 기능 추가

921154d

feat(Piece): 각 기물의 종류를 구분하는 기능 추가

2d39066

feat(OutputView): 장기판 출력 기능 추가

216581f

feat(OutputView): 팀 색깔 구분 기능 추가

51e5c82

refactor: 기물이 가진 팀을 묻도록 리팩터링

d3021ac

feat(Wang): 왕의 이동 가능 여부 확인 기능 추가

c72d5d9

feat(Sa): 사의 이동 가능 여부 확인 기능 추가

c1b5611

fix: 노드 엣지 초기화 기능 수정

af7b771

feat(Byeong): 병의 이동 가능 여부 확인 기능 추가

edbd206

feat(Cha): 차의 이동 가능 여부 확인 기능 추가

6ce8aff

feat(Ma): 마의 이동 가능 여부 확인 기능 추가

225fe9d

feat(Sang): 상의 이동 가능 여부 확인 기능 추가

c39fbe5

refactor: 변수명 개선, 인덴트 깊이 개선

5473e7c

refactor: 가독성 개선

13db4fa

fix: 테스트 코드 컴파일 에러 수정

d88377a

feat(Po): 포의 이동 가능 여부 확인 기능 추가

fea65fd

feat: 상마 순서 입력 기능 추가

11b48a6

feat: 이동할 기물의 위치 입력 기능 추가

1f87130

threepebbles added 20 commits March 23, 2025 16:26

refactor: 네이밍 개선

38ad5bc

refactor(Piece): canMove에서 파라미터 타입을 Node가 아닌 Point를 받아서 기능을 수행하도록 수정

9fec375

refactor: 클래스명 개선

0d1c4dc

test: board가 아니라 각 piece 객체가 canMove()를 수행하도록 수정

7c66006

refactor(Path): Path 클래스 enum 상수화

7edb1a6

refactor(Piece): 파라미터로 팀을 확인하는 기능 hasTeam() 제거, 팀 enum을 반환하는 기능 team(…

a0dd32c

…) 추가

test(NodeTest): 노드 테스트 추가

6d9ddd9

refactor: exist -> exists 용어 통일

6c59923

refactor: 패키지 이동

10a4769

refactor(Direction): 상하좌우 방향 리스트(VERTICALS) 상수화

5ed393d

fix: 상수에 final 키워드 추가

db21822

refactor: 상마 순서 예외 메시지 개선

679785f

refactor: 게임 종료 시 승리한 팀을 찾는 기능 옮김(JanggiManager -> Board)

0ff42ef

feat(PointNodeMapperFactory): PointNodeMapper의 생성 책임을 클래스로 분리

ad6a041

refactor: 메서드 파라미터에 final 키워드 추가

f04434e

refactor: 입출력 메시지 개선

6060d63

refactor(Node): 불필요한 필드(Point) 제거

1d0f997

test(NodeTest): 출발 노드로부터 간선이 있는 방향은 true, 없는 방향은 false를 반환하는 기능 테스트 추가

ccb2f61

refactor: 동일한 로직 메서드 분리(DRY 원칙)

4668962

refactor: 파라미터에 final 키워드 추가

5e26b6c

threepebbles added 6 commits March 24, 2025 09:52

refactor: 메서드명 개선

1bffd19

refactor: 변수 네이밍 일관되게 수정(board -> pieceByPoint)

6999e71

refactor: 가독성을 위해 메서드 분리

0b7bc22

refactor: 가독성 개선을 위해 에러 메시지에 줄바꿈 추가

3f6ea82

fix: 기물을 움직일 수 없는 경우에 턴이 바뀌지 않도록 수정

f3838b9

refactor: 보드판 출력 시기 수정

f92a46e

Rok93 approved these changes Mar 24, 2025

View reviewed changes

Rok93 merged commit 68a37bb into woowacourse:threepebbles Mar 24, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[1단계 - 장기] 헤일러(민서현) 미션 제출합니다. #10

[1단계 - 장기] 헤일러(민서현) 미션 제출합니다. #10

threepebbles commented Mar 20, 2025 •

edited

Loading

threepebbles commented Mar 23, 2025 •

edited

Loading

threepebbles commented Mar 24, 2025 •

edited

Loading

Rok93 left a comment

Rok93 Mar 24, 2025

Rok93 Mar 24, 2025

threepebbles Mar 25, 2025

threepebbles Mar 25, 2025

Rok93 Mar 24, 2025

threepebbles Mar 25, 2025

Rok93 Mar 24, 2025

Rok93 Mar 24, 2025

threepebbles Mar 25, 2025

Rok93 Mar 24, 2025

Rok93 Mar 24, 2025

Rok93 Mar 24, 2025

Rok93 Mar 24, 2025

Rok93 Mar 24, 2025

	return findTeamsOfWang().containsAll(List.of(Team.CHO, Team.HAN));
	findTeamsOfWang().size() == 2;

[1단계 - 장기] 헤일러(민서현) 미션 제출합니다. #10

[1단계 - 장기] 헤일러(민서현) 미션 제출합니다. #10

Conversation

threepebbles commented Mar 20, 2025 • edited Loading

체크 리스트

객체지향 생활체조 요구사항을 얼마나 잘 충족했다고 생각하시나요?

1~5점 중에서 선택해주세요.

선택한 점수의 이유를 적어주세요.

어떤 부분에 집중하여 리뷰해야 할까요?

📌 노드와 엣지를 이용한 인접 리스트 구현 방식

📌 기물들의 움직임 패턴과 관련된 상수 관리

threepebbles commented Mar 23, 2025 • edited Loading

💬 코멘트에 대한 답변

인접 리스트 방식을 사용한 이유 (노드, 엣지)

Point(포인트)

Node(노드)

Edge(엣지)

❓ 여쭤보고 싶은 점

1️⃣ "상"과 "마"의 움직임 관리

2️⃣ 계층에 따라 일관되게 예외 처리하는 것이 괜찮을까요?

3️⃣ 전체적으로 방어적 검증(?)이 과하지는 않은지 피드백 받고 싶습니다.

4️⃣ 생성자에 넣기에는 코드의 양이 너무 많을 때 어떻게 하시나요?

threepebbles commented Mar 24, 2025 • edited Loading

Rok93 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

threepebbles commented Mar 20, 2025 •

edited

Loading

threepebbles commented Mar 23, 2025 •

edited

Loading

threepebbles commented Mar 24, 2025 •

edited

Loading