[1, 2단계 - 체스] 제이(이재윤) 미션 제출합니다. #439

sosow0212 · 2023-03-16T08:17:26Z

카프카, 안녕하세요! 제이입니다.

미션을 진행하면서

처음에 체스 말에서 Position을 가지게 했습니다.

아코와 함께 고민한 끝에 Position은 체스 말이 가지기는 부적절하다고 생각했습니다.(체스 말이 스스로 움직이는 것이 아닌 사람이 체스 말을 움직인다고 생각했기 때문입니다!)

따라서 체스 말은 목적지의 위치가 주어졌을 때, 그 위치로 갈 수 있는 기능이 있는지에 대해서만 알고 있도록 구현하였습니다.

체스 보드에서 Map 자료구조를 사용했고, Key 값으로 Position을 가지고 Value 값으로 Piece(체스 말) 가지게 리팩토링을 했습니다.
또한 체스는 다양한 경우가 있기 때문에, 페어인 아코와 함께 여러 경우를 테스트하려고 노력했습니다.

궁금한 점

현재 미션에서 Map의 Key값으로 Position을 사용하고 있습니다. 하지만 생각보다 Position이 하는 역할이 없는 것 같습니다. (그냥 String으로 두어도 될 것 같아요!)

“그래서 이걸 그냥 지워야하나?” 라는 고민을 많이 했습니다.
그리고 현재 Piece(기물)을 상속받아서 체스 말과 빈 공간을 구현했습니다.

빈 공간은 엄밀히 말하면 기물은 아니지만 체스를 구성하는 요소라고 생각해서 위와 같이 구현했습니다. 행위에 주체가 되는 체스의 말과 기물을 같은 선상에 두는 일에 대해 어떻게 생각하시나요?
추상 클래스(기물 Piece)에서 caMove() 추상 메서드를 만들어놓고 기물들이 이를 상속받아서 씁니다.

각 기물에는 해당하는 역할에 맞게 목적지에 갈 수 있는지 검증을 하고, 검증이 통과가 된다면

Board 도메인 안에 switchPosition()을 실행시켜 말의 위치를 이동시킵니다.

여기서 문제점이, 이동하는 경우의 예외를 테스트를 할 때 switchPosition() 메서드를 너무 많이 의존하게 됩니다. 카프카는 이에 대해서 어떻게 생각하시나요?

소중한 시간 내주셔서 감사합니다 :)

세부사항: 추후에 Position은 Map의 좌표값으로 사용할 예정

세부사항 이동 방법에 대한 설명 주가

include42 · 2023-03-16T14:04:26Z

안녕하세요, 제이! 리뷰어 카프카입니다.
작업하시느라 고생 많으셨습니다. 리뷰는 내일(17일/금) 오전 10시 전후로 진행 예정입니다.
푹 쉬시고, 함께 즐겁게 논의하며 성장하면 좋겠습니다 💪🏻

include42

안녕하세요 제이! 리뷰어 카프카입니다.

체스 게임 미션이 많이 어려웠지요? 그래도 잘 구현해 주셔서, 즐겁게 리뷰할 수 있었습니다.
개선할만한 부분에 코멘트 남겼는데, 질문주신 내용들도 함께 언급해 두었으니 확인 부탁드립니다.

작업하시느라 고생 많으셨습니다 💪

include42 · 2023-03-17T00:17:05Z

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

+        if (!(piece instanceof Knight)) {
+            validateObstacle(start, end);
+        }


instanceof를 안쓰는 방향으로 개선할 수 있다면 좋을 것 같습니다.
Piece 클래스에 해당 말이 다른 말을 건너뛸 수 있는지 여부를 boolean method로 정의하면 어떨까요?

instance of를 이용해서 바깥쪽에서 해줄 필요 없이
도메인 내부 메서드로 해결할 수 있었네요!

감사합니다👍

include42 · 2023-03-17T00:19:33Z

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

+        return board.get(Position.from(start));
+    }
+
+    public void switchPosition(final String start, final String end) {


switchPosition을 수행할 때 start와 end를 String으로 전달하는 이유가 있을까요?
ChessGame 클래스에서 Position으로 변경하고 줘도 될 것 같고 그 편이 명세나 검증 면에서도 나아보이는데,
혹시 다른 이유가 있을지 궁금해서 코멘트 남깁니다.

살펴보니 Direction을 생성할 때에도 그렇고, 내부적으로 String으로 포지션 정보를 많이 가지고 있네요.
이를 최대한 Position 클래스로 변경해보는게 좋을 것 같습니다. 궁금한 점에도 적어주셨었는데...
Position을 사용할 경우 생성과정에서 위치값(String)이 올바른지 검증할 수 있고, 값을 포장해서 잘못 사용되는 것을 막을 수 있으니까요. 이부분은 Position이 사용되는 곳들을 살펴보면서 고민을 해볼 필요가 있어보입니다.

기존 코드는 움직임을 테스트를 할 때 가장 깊은 곳에서 String에서 Position으로 파싱이 될 때 움직일 수 있는 영역인지 예외 처리를 하게 됩니다.

말씀해주신대로 Position으로 변경을 먼저 한다면, 기존 Position 객체를 더 많이 활용 할 수 있고(계산이 편해지네요!), 위치 값을 포장된 객체로 넘기면 잘못 사용될 일도 적어질 것 같습니다!

감사합니다 :)

include42 · 2023-03-17T00:26:36Z

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

+    }
+
+    private void validatePawnMove(final Piece piece, final String start, final String end) {
+        if (!(piece instanceof Pawn)) {


Pawn 여부를 많이 검사하고 이를 바탕으로 검증을 수행하게 되는데요,
이부분도 마찬가지로 isPawn 메소드를 Piece 클래스에 미리 만들어둬서 그것으로 검증하는 것이 좋지 않을까요?

관련된 좋은 글이 tecoble에 있어, 링크 남겨둡니다.
https://tecoble.techcourse.co.kr/post/2021-04-26-instanceof/

좋은 자료 감사합니다!

instance of를 사용하면서 "굉장히 편하다"라는 생각을 했는데, 좋은 방법은 아니었군요 ㅠㅠ
보내주신 자료 참고해서 정리하도록 하겠습니다!

include42 · 2023-03-17T00:35:36Z

src/main/java/chess/domain/pieces/King.java

+    public void canMove(final String start, final String end) {
+        List<List<Integer>> possiblePosition = List.of(List.of(1,0), List.of(0,1), List.of(1,1));
+
+        int absSubRow = Math.abs(Row.subPositionFromArrivePosition(start.charAt(ROW), end.charAt(ROW)));
+        int absSubCol = Math.abs(Col.subPositionFromArrivePosition(start.charAt(COLUMN), end.charAt(COLUMN)));
+
+        List<Integer> subPosition = List.of(absSubRow, absSubCol);
+
+        if (!possiblePosition.contains(subPosition)) {
+            throw new IllegalArgumentException("올바른 위치가 아닙니다.");
+        }
+    }


이부분은 코드의 가독성이 조금 더 개선되면 좋겠네요.
말의 움직임에 대해 어떻게 검증을 하고 있는 것인지 이해에 시간이 좀 걸렸습니다.

include42 · 2023-03-17T00:41:53Z

src/main/java/chess/domain/pieces/Place.java

@@ -0,0 +1,15 @@
+package chess.domain.pieces;
+
+public class Place extends Piece {


Place는 공백 개념을 클래스로 만든 것이지요?
관련 질문도 주셨었는데, 제 생각엔 이렇게 만든 것이 좋아 보입니다.
공백에 대해 필요한 처리들을 여기서 정의해둬서, 간편한 부분이 많아 보이네요 💯

저는 Piece(기물)이라서 Place(공백)가 기물은 아니니 들어가도 되는지에 너무 궁금했는데, 계속 고민해보니 공백칸도 어떻게 보면 체스의 말은 아니지만 체스를 이루는 기물이라고 생각해서 다음과 같이 만들었습니다!

감사합니다 🙇🏻‍♂️

기물이라는 면에서 보면 애매할수도 있지만, 해당 클래스가 없을 경우 공백 출력이나 위치교환 처리가 복잡해질 염려가 있어 보였습니다.
다만 Place라는 이름이 적절한지는 고민이 되네요. Blank라던가, 공백이라는 의미의 이름을 사용해도 좋을 듯 싶습니다.

include42 · 2023-03-17T00:42:50Z

src/main/java/chess/domain/pieces/Queen.java

+    private void validateMove(final String start, final String end) {
+        int subRow = Row.subPositionFromArrivePosition(start.charAt(ROW), end.charAt(ROW));
+        int subCol = Col.subPositionFromArrivePosition(start.charAt(COLUMN), end.charAt(COLUMN));
+
+        if (!validateMoveLikeBishop(subRow, subCol) && !validateMoveLikeRook(subRow, subCol)) {
+            throw new IllegalArgumentException("Queen의 이동 범위가 올바르지 않습니다.");
+        }
+    }
+
+    private boolean validateMoveLikeBishop(final int subRow, final int subCol) {
+        return Math.abs(subCol) == Math.abs(subRow);
+    }
+
+    private boolean validateMoveLikeRook(final int subRow, final int subCol) {
+        return subRow == 0 || subCol == 0;
+    }


앞서 King의 canMove 부분이 아쉽다고 코멘트를 남겼는데, Queen 부분은 메소드를 잘게 분리해놔서 오히려 이해하기 쉬웠네요. 👍

include42 · 2023-03-17T00:46:28Z

src/main/java/chess/factory/BoardFactory.java

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


BoardFactory의 코드가 전반적으로 깔끔하네요! 😄

include42 · 2023-03-17T00:51:36Z

src/test/java/chess/domain/board/BoardTest.java

+    void throws_exception_when_move_invalid(final String start, final String end) {
+        // given
+        Board board = BoardFactory.createBoard();
+
+        // when & then
+        assertThatThrownBy(
+                () -> board.switchPosition(start, end)
+        ).isInstanceOf(IllegalArgumentException.class);


BoardTest를 잘 작성해 주셨습니다.
switchPosition에 대해 테스트에서 의존이 생긴다고 걱정해 주셨는데, private한 validate 로직들을 테스트하기 위해 해당 메소드를 호출하는 것은 자연스러운 흐름이라고 생각합니다.

다만 assertThatThrownBy로 검증할 때, 예외 메시지까지 함께 검증하면 더 좋지 않을까요?

말씀해주신대로 예외 메시지까지 검증하는 것이 맞다고 생각합니다. (하나의 메서드를 작동 할 때, 여러 예외 중에서 어떤 예외가 터졌는지 한 번 더 체크를 할 수 있기 때문입니다.)

Enum을 이용해서 검증하도록 하겠습니다. 감사합니다~~

include42 · 2023-03-17T00:56:05Z

src/test/java/chess/domain/board/PositionTest.java

+    @Test
+    @DisplayName("정상적인 위치면 예외를 발생시키지 않는다.")
+    void throws_not_exception_when_case_is_normally() {
+        // when & then
+        assertDoesNotThrow(
+                () -> Position.from("b2")
+        );
+    }


저의 경우, 정상 동작하는지 확인하는 테스트 작성히 해당 클래스를 직접 만들고, 내부값을 get 해보는 것 까지 수행하는 편입니다.
위의 경우, Position.from("b2")를 변수로 선언한 뒤, 해당 변수에서 getRow와 getCol을 한 값이 b2와 일치하는지 본다는 의미입니다.

이렇게 테스트를 작성하는 게 불필요하다고 생각할 수도 있습니다. 다만 단순히 throw가 없는 것만 확인한다면, 생성 로직이 변경되어 잘못된 형태로 생성되더라도 검증하기가 어렵다고 생각하니까요. 수정을 고려해보시면 좋을 것으로 생각됩니다.

말씀하신대로 단순히 생성시 오류가 안나는지 검증하는 것 뿐만 아니라 의도한 값까지 정상적으로 생성되는지 확인하겠습니다!

테스트를 짜는 건 금방이지만, 검증은 계속 필요하니 앞으로 일치하는 것까지 확인하는게 좋을 것 같습니다.

감사합니다 🙏

include42 · 2023-03-17T01:00:15Z

src/test/java/chess/domain/pieces/PawnTest.java

+
+    @ParameterizedTest
+    @CsvSource(value = {"c7,c6", "c7,c5"})
+    @DisplayName("폰이 올바른 위치로 움직인다.")


DisplayName에 적어준 텍스트가 위의 다른 테스트와 겹치네요.
그리고 해당 텍스트와 포지션 정보가 함께 노출되면 좋을 것으로 생각되는데요,
https://stackoverflow.com/questions/650894/changing-names-of-parameterized-tests
ParameterizedTest 사용시 DisplayName 대신 name 필드를 부여하는 방법에 대한 글입니다.
참고해서 보기 좋게 테스트 명세에 반영해보는걸 추천드립니다.

와.. 이거 진짜 완전 꿀팁이네요!
유용하게 잘 쓰겠습니다 👍👍

--> Col -> Column

include42 · 2023-03-17T08:33:57Z

안녕하세요 제이! 리뷰하면서 피드백 사항이 꽤 되었는데 바로 확인해 주셨네요 💯
리뷰는 내일 오전 10시 전후로 가능할 것으로 보입니다. 혹시 늦어지게 되면 코멘트 남겨두겠습니다.
작업하시느라 고생 많으셨습니다. 주말동안 푹 쉬시고, 즐겁게 논의 이어나갔으면 좋겠습니다 😄

include42

안녕하세요 제이, 리뷰어 카프카입니다.
저번 리뷰에서 피드백을 빠르게 반영해 주셨네요. 💯
전체적인 구조가 안정적이라, 즐겁게 리뷰할 수 있었습니다.

아직 몇몇 부분에서 개선점이 존재하지만, 해당 부분은 다음 스탭을 진행하면서 고쳐도 될 것으로 생각됩니다.
이번 리뷰 과정에서 현재 스탭의 요구조건을 잘 충족했다고 생각되어, 미션 approve 하겠습니다.

작업하시느라 고생 많으셨습니다. 다음 단계에서 다시 뵙겠습니다 😄

include42 · 2023-03-17T15:08:44Z

src/main/java/chess/controller/ChessGameController.java

+    private ChessGame createNewChessGame(ChessGame chessGame, final List<String> inputCommand) {
+        if (inputCommand.get(COMMAND_INDEX).equals(START_COMMAND)) {
+            chessGame = new ChessGame(BoardFactory.createBoard());
+        }
+        return chessGame;
+    }


프로그램을 다시 테스트해보니, 처음에 start를 입력하지 않고 바로 move를 입력해도 게임이 동작하더라고요.
왜 그런가 살펴보니, start를 입력할 경우 chessGame을 생성한 채로 주입해주고 있네요.
여기서 새로 ChessGame 클래스를 생성해주지 않더라도, 게임 진행에는 문제가 없도록 설계되어 있고요.

다만 이 구조 때문에, 게임 진행 도중 start를 입력하면 게임이 재시작되는 걸 확인할 수 있었습니다.
이러한 구조가 명세에 맞다고 생각되면 문제가 없겠으나, 의도된 부분이 아니라면 로직을 개선할 필요가 보입니다.
ChessGame을 생성하는 부분과, 생성한 ChessGame 객체로 입력을 받아 게임을 진행하는 부분이 별도의 메소드로 나뉘어야 할 듯 해요.

그리고 InputCommand가 일급 컬렉션이 되어도 좋을 것으로 생각됩니다.
현재처럼 get으로 참조하는게 아니라, 내부 메소드로 타입을 얻어오는 메소드와 파라미터를 얻어오는 메소드를 만들면 되니까요.

이 경우 해당 일급 컬렉션의 생성 단계에서 validate를 진행할 수 있을테니, 잘못된 입력에 대해서 InputViewValidator가 검증하지 못하는 부분(예: 포지션의 범위 벗어남이나 START/END 코맨드인데 인자가 추가로 있는 경우 등...)에 대해서도 더 세밀히 검증할 수 있겠네요

이부분도 고려해보고 여유가 된다면 적용해보기를 바랍니다.

페어와 처음에 만들 때 2단계에서 start를 입력하면 재시작 하게 끔 의도하고 만들었습니다.

다만 맨 처음 단계에서 move 커맨드는 막아둘 필요가 있어보입니다.
일급 컬렉션으로 만드는 것도 훨 좋아보이네요 참고 하겠습니다 :)

감사합니다!

include42 · 2023-03-17T15:20:42Z

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

+    private void validateLowercasePawnAttack(final Position end) {
+        Piece upperEnemy = findPieceFromPosition(end);
+        if (upperEnemy.isNameLowerCase() || upperEnemy.isPlace()) {
+            throw new IllegalArgumentException(PieceMessage.PAWN_INVALID_MOVE.getMessage());
+        }
+    }
+
+    private void validateLowercasePawnMoveForward(final Position end) {
+        Piece destination = findPieceFromPosition(end);
+        if (!(destination.isPlace())) {
+            throw new IllegalArgumentException(PieceMessage.PAWN_INVALID_MOVE.getMessage());
+        }
+    }


Board 내의 인자들일 모두 Position으로 변경되어서, 가독성이 많이 좋아졌네요 💯
validate시 사용될 에러 메시지를 별도로 만들어둔 부분도 매우 좋았습니다.

혹시 custom exception class를 만들어 본 적이 있을까요? 만약 그렇다면, 해당 메시지 타입을 인자로 받는 예외 클래스를 직접 구현해 보는 것도 좋은 경험이 될 것 같네요. 다음 스탭을 진행하면서 여유가 되면 시도해보기를 바랍니다. (필수는 아닙니다. 그것이 좋다고 생각된다면, 적용해보는 것을 권합니다)

사실 기존에 스프링 공부를 했을 땐 CustomException이 예외에 대한 응집도 및 클래스 명 자체로 에러 정보를 전달할 수 있다는 점에서 좋다고 느꼈습니다.

하지만, 이 방법에서는 표준 예외로 전달할 수 있지만 비용을 써야한다는 점 (물론 비용이 크진 않지만요..!)과 클래스의 수가 많아진다는 점에서 현재 미션에서 사실은 꼭 안 써도 된다고 생각합니다!

사실 아직도 "Enum으로 예외 메시지를 전달해주는 방법이 더 좋은가?"에 대해서는 엄청 좋다라고 느끼지는 못하고 있습니다.

기존에 Custom Exception이 익숙해서 그런가 싶기도 합니다.
따라서 이번 미션까지만 Enum으로 예외를 관리하면서 직접 느껴보고 추후에 "어떤 점에서 어떤 것이 더 좋을 수 있다!"라는 주제로 포스팅 해보겠습니다 :)

좋은 의견 주셔서 감사합니다!

확실히 Custom Exception은 구현 리소스 뿐 아니라, 코드를 보는 사람이 해당 클래스를 확인해야 한다는 점에서도 비용의 문제가 있습니다. 다만 생성자나 정적 팩토리 메소드를 통해 로깅 혹은 메시지 가공을 수행할 수 있다는 점에서의 장점이 있겠지요.

고민하고 있는 내용에 대해 포스팅해두면 두고두고 큰 도움이 됩니다! 제이의 성장을 응원하겠습니다 💯

include42 · 2023-03-17T15:28:48Z

src/main/java/chess/domain/direction/Direction.java

+    private static final int ROW = 1;
+    private static final int COLUMN = 0;


이 두 값은 미사용되는 값으로 보이는데, 삭제해두면 좋을 것으로 생각됩니다.

include42 · 2023-03-17T15:29:41Z

src/main/java/chess/domain/direction/Direction.java

+        return Arrays.stream(Direction.values()).
+                filter(direction -> direction.isSameDirection(start, end))
+                .findFirst()
+                .get();


값이 없는 경우에 대해 orElseThrow를 수행해줘야 하는 것으로 보입니다.
해당 로직이 없어서, InteliiJ에서 warning을 띄워주고 있네요.

include42 · 2023-03-17T15:31:48Z

src/main/java/chess/domain/pieces/Bishop.java

+        int absSubRow = Math.abs(Row.subPositionFromArrivePosition(start.getRow(), end.getRow()));
+        int absSubCol = Math.abs(Column.subPositionFromArrivePosition(start.getCol(), end.getCol()));
+
+        if (!(absSubCol == absSubRow)) {


이부분은 absSubCol != absSubRow 라고 표현해도 같은 의미가 되지 않을까요?

include42 · 2023-03-17T15:35:46Z

src/main/java/chess/domain/pieces/Piece.java

+    public boolean isPawn() {
+        return this.name.isPawn();
+    }


name에 isPawn과 isKnight가 정의되어 있어서 의아했는데, 이 부분 로직 때문에 그렇네요.
여기서는 그냥 false를 return해주고, Pawn이 이 메소드를 override해서 true를 return해주면 되지 않을까요?

이런 방법이 있었군요..!

코드를 보면서 생각해보니 한가지 의문이 생겼습니다.
상속을 이용하면서 각각의 체스 말을 구분하면 다형성에 벗어나지 않을까요?

카프카는 이에 대해 어떻게 생각하시나요?

저도 그 부분을 고민 많이 했던 기억이 나네요.
물론 다형성을 엄격히 따진다면, 이 부분이 아쉬울 수 있습니다. 하지만 충분히 타협할 수 있는 정도라고 저는 생각합니다.
체스 기물(Piece)과 그 상속 클래스들의 명세가 명확하고, 기획의 변경 여지가 적어서 이러한 메소드가 더 늘어날 걱정은 없어보이기 때문입니다. 또한 해당 메소드를 상속하지 않고 호출시 exception을 발생시키거나 false를 발생시키는 것으로 어느정도 잘못된 사용을 방지할 수 있고요.

좀 더 엄격하게 구현한다면, Piece는 enum으로 된 PieceType(PAWN, KNIGHT, KING...)을 getPieceType 메소드로 반환하고, 해당 타입을 받아 필요한 곳에서 검증할 수 있겠습니다.

include42 · 2023-03-17T15:41:54Z

src/main/java/chess/view/OutputView.java

+    public void printBoard(final Map<Position, Piece> board) {
+        System.out.println();
+        for (char row = '8'; row >= '1'; row--) {
+            printLine(board, row);
+            System.out.println();
+        }
+    }
+
+    private void printLine(final Map<Position, Piece> board, final char row) {
+        for (char col = 'a'; col <= 'h'; col++) {
+            String position = String.valueOf(col) + String.valueOf(row);
+            System.out.print(board.get(Position.from(position)).getName());
+        }
+    }


이부분이 약간 아쉽네요. for문을 통해 순회하지 않아도 되도록 개선하면 좋을텐데 말이죠.
혹시 Board에 현재 판의 정보를 String 객체에 저장하는 메소드를 추가하고(여기서 수행하는 것보다 깔끔하고, 책임도 조금 더 명확해지지 않을까 생각됩니다) 해당 값을 OutputView에 전달해주면 어떨까요?

이 경우 장점도 단점도 명확해질텐데, 일단 View가 도메인 정보에 너무 의존하지 않고 간결히 역할을 수행하도록 고민해보는 것입니다.
이것이 좋다고 생각한다면 추후 반영해보시고, 현재 구조가 더 좋다면 유지해도 괜찮습니다.

미션을 진행하면 할 수록 이 부분이 사실 너무 헷갈립니다..

도메인에서 계산을 한 것을 View로 바로 전달해서 깔끔하게 출력한다.

도메인은 순수 필드 값만 주고, View에서 계산하고 처리한다.

도메인과 View 사이에 Dto를 만들어서 Dto에서 계산을 하고 View에게 준다.

전부 다 장단점이 있다고 생각합니다.

저는 개인적으로 "콘솔에서 출력이 오류가 있다는 것을 발견하면 어디로 가서 리팩토링을 할 것인가?" 라는 생각을 한다면 View로 갈 것 같습니다.
즉 저는 출력에 관련된 로직 계산은 View에서 하는게 조금 더 명확하다고 생각했습니다.

이 생각에 대해 카프카는 어떻게 생각하시나요?
저는 미션을 할 수록 이 고민에 대해서 어떤 것이 조금 더 명확한지에 대해 너무 고민이 많습니다.

이 부분에서 고민하는 것이 당연하고, 또 좋다고 저는 생각합니다.
앞으로 level 2를 진행하고 프로젝트를 진행하다 보면 보다 로직이 분명한 frontend 뷰 레이어와 통신을 하게 될 텐데요.
물론 그때는 DTO를 통해서 값을 전달하는게 필수지만, 값을 어느정도 가공해서 전달할지에 대해 고민이 많아질 것입니다.
이 문제는 정답이 없기 때문에, 앞으로 미션을 진행하면서 자신만의 방식을 만들어가는 것이 중요합니다. 그 과정에서 다양한 의견을 청취해야 겠고요.

이 미션의 경우, View 레이어의 역할이 애매하기 때문에 더 고민이 될 것으로 보여요.
만약 View 레이어를 단순히 print만 해주는 수동적 역할로 본다면, 계산한 값을 DTO에 넣어 보내는 것이 이상적이라고 생각합니다.
그러나 제이의 의견처럼 View가 좀더 능동적으로 출력 동작을 책임진다면, 필드값을 주는 것이 더 올바르다고 할 수 있겠지요.

제 생각엔 일단 다음 스탭 진행 과정에서 DTO를 적극적으로 도입해보고, 그 과정에서 View의 역할을 적당한 정도까지 맞춰나가는 것이 좋아보입니다. 만약 그 과정에서 전달하는 값이 너무 번잡하고 View의 로직이 길어지면, 어느정도 DTO를 만드는 과정에서 데이터를 미리 가공해 보낼 수 있겠지요.

include42 · 2023-03-17T15:44:51Z

src/test/java/chess/view/InputViewValidatorTest.java

+
+class InputViewValidatorTest {
+
+    @ParameterizedTest


전반적으로 테스트 개선을 잘해주셨습니다 💯
여기 InputViewValidatorTest에서만 추가로 ParameterizedTest 내에 name 옵션 사용해주시면 좋을 것으로 보이네요.
추가로, 정상 케이스에 대해 validate가 성공하는 테스트를 하나 추가해 봅시다 👍

sosow0212 added 30 commits March 14, 2023 15:08

docs: 구현할 기능 목록 추가

8d38f00

feat: 수업 강의 시간에 DB 연동 구현

b5bd484

feat: 위치 생성하는 기능 추가

8827783

feat: 기물 및 빈 공간 예외 사항 구현

8366023

feat: Knight의 이동 로직 구현 및 예외 추가

4a85c31

refactor: 절댓값을 반환하지 않고 목표 지점에서 현재 포지션의 차이값만 반환해주게 리팩토링 진행

e1bd694

feat: 폰의 움직이는 기능 구현

48dde74

feat: Rook의 움직이는 기능 구현

975e0b5

refactor: 중복 메서드 제거 및 Position 객체의 역할 추가

5bb0ab1

feat: king의 움직이는 기능 구현

372af8d

feat: bishop의 움직이는 기능 구현

7523917

refactor: 불필요한 메서드 제거

2662278

feat: Queen의 움직이는 기능 구현

f1cdfbb

refactor: Piece가 Position을 갖지 않도록 리팩토링 진행

4648308

세부사항: 추후에 Position은 Map의 좌표값으로 사용할 예정

docs: 기능 구현 목록 추가

8d28cd0

feat: 체스판 구현

d210fe8

docs: 기능 목록 구현 수정

eeb4a0c

feat: 게임 명령어 입력 받는 기능 구현

55ce416

refactor: pawn에서 팀이 다른 경우 움직이는 경우의 수 추가

9ae6839

style: 안쓰는 import 제거

1a40b5b

refactor: Place 변경에 따라 리팩토링 진행

3c0eeef

test: pawn이 두 번 움직이는 경우 예외 테스트

c002ff7

feat: board 에서 체스 말 움직임 기능 추가

1371a4d

feat: 출발 위치와 도착 위치 사이의 경로를 반환하는 기능 추가

3598619

feat: 경로에 장애물(같은 팀, 상대 팀 인식 포함)이 있는지 검증하는 기능 추가

d2f4243

feat: 체스 게임 관리하는 도메인 추가

99127c0

feat: 체스게임 실행 로직 추가

763c9a1

docs: 기능 목록 구현 수정

f4b1f99

docs: 기능 목록 구현 수정

17559e3

세부사항 이동 방법에 대한 설명 주가

refactor: 폰의 대각선 움직임 고려 및 예외추가

8d0f6a9

sosow0212 added 2 commits March 16, 2023 16:34

refactor: 상수 추출 및 개행 리팩토링

4023592

refactor: 매직 넘버 및 매직 리터럴 상수 처리

599068f

include42 requested changes Mar 17, 2023

View reviewed changes

sosow0212 added 11 commits March 17, 2023 12:07

feat: stream & lambda 미니미션

43b55f4

refactor: Position 객체를 활용해서 움직이게 로직 변경

cde4a2a

refactor: instance of 사용 대신 도메인 메서드를 통해 체스말의 종류가 무엇인지 검증하도록 리팩토링 진행

8b6593d

refactor: King의 이동 예외 코드를 보다 명확하게 네이밍 및 메서드명 리팩토링 진행

e5cf972

refactor: King의 이동 예외 코드를 보다 명확하게 네이밍 및 메서드명 리팩토링 진행

86fb692

refactor: 객체의 이름을 줄여쓰는 것이 아닌 풀어서 사용하도록 리팩토롱

06bb626

--> Col -> Column

refactor: 체스 말의 예외 메시지 검증 메시지 분리 및 테스트에서도 메시지 검증하도록 리팩토링 진행

3fda80e

refactor: 체스 말의 예외 메시지 검증 메시지 분리 및 테스트에서도 메시지 검증하도록 리팩토링 진행

9653f9b

refactor: 테스트 값 검증 및 메시지 검증 및 테스트 분리

d240b7d

refactor: @ParameterizedTest에 이름을 붙여서 테스트를 더욱 명확히 볼 수 있도록 변경

49df6c0

refactor: 예외 메시지 Enum 처리

741605e

include42 approved these changes Mar 17, 2023

View reviewed changes

include42 merged commit c10854a into woowacourse:sosow0212 Mar 17, 2023

		@@ -0,0 +1,15 @@
		package chess.domain.pieces;

		public class Place extends Piece {

		private static final int ROW = 1;
		private static final int COLUMN = 0;

[1, 2단계 - 체스] 제이(이재윤) 미션 제출합니다. #439

[1, 2단계 - 체스] 제이(이재윤) 미션 제출합니다. #439

Conversation

sosow0212 commented Mar 16, 2023 • edited

미션을 진행하면서

궁금한 점

include42 commented Mar 16, 2023

include42 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

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

include42 commented Mar 17, 2023

include42 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sosow0212 commented Mar 16, 2023 •

edited