[1, 2단계 - 체스] 다즐(최우창) 미션 제출합니다. #488
Conversation
Co-authored-by: swonny <wonny921@gmail.com>
Co-authored-by: swonny <wonny921@gmail.com>
Co-authored-by: swonny <wonny921@gmail.com>
Co-authored-by: swonny <wonny921@gmail.com>
Co-authored-by: swonny <wonny921@gmail.com>
Co-authored-by: swonny <wonny921@gmail.com>
Co-authored-by: swonny <wonny921@gmail.com>
Co-authored-by: swonny <wonny921@gmail.com>
Co-authored-by: swonny <wonny921@gmail.com>
Co-authored-by: swonny <wonny921@gmail.com>
Co-authored-by: swonny <wonny921@gmail.com>
Co-authored-by: swonny <wonny921@gmail.com>
Co-authored-by: swonny <wonny921@gmail.com>
Co-authored-by: swonny <wonny921@gmail.com>
Co-authored-by: swonny <wonny921@gmail.com>
Co-authored-by: swonny <wonny921@gmail.com>
Co-authored-by: swonny <wonny921@gmail.com>
Co-authored-by: swonny <wonny921@gmail.com>
Co-authored-by: swonny <wonny921@gmail.com>
Co-authored-by: swonny <wonny921@gmail.com>
Co-authored-by: swonny <wonny921@gmail.com>
Co-authored-by: swonny <wonny921@gmail.com>
Co-authored-by: swonny <wonny921@gmail.com>
Co-authored-by: swonny <wonny921@gmail.com>
Co-authored-by: swonny <wonny921@gmail.com>
Co-authored-by: swonny <wonny921@gmail.com>
Co-authored-by: swonny <wonny921@gmail.com>
Co-authored-by: swonny <wonny921@gmail.com>
Co-authored-by: swonny <wonny921@gmail.com>
Co-authored-by: swonny <wonny921@gmail.com>
Co-authored-by: swonny <wonny921@gmail.com>
|
리뷰 해주신 내용을 바탕으로 리팩토링을 진행해 보았습니다 ⭐️
다시 한번 리뷰 부탁드립니다 감사합니다! |
| import chess.domain.board.Square; | ||
| import java.util.List; | ||
|
|
||
| public class ChessGameCommand { |
There was a problem hiding this comment.
현재 명령어는 start, end, move 로 한정되어 있고, 이 각각의 명령어를 판단을 현재는 Controller의 Command 가 하고 있습니다.
이러한 명령어를 입력하는 방식이 변경된다면, 예를 들어서 START, END, MOVE 대문자로 or 버튼 클릭 방식으로 변환된다고 했을 때, 이를 지금 구조에서 어떤 방식으로 반영할 수 있을까요?
뷰와 도메인 사이에서 의존관계를 적절히 처리하기 위한 역할을 컨트롤러가 한다고 생각합니다.
제 개인적인 생각을 남기면, Controller는 그 interface 가 명확하게 정의되어 있고 다양한 형태의 view가 이 interface를 구현해야 한다 생각합니다.
| public enum ChessStartCommand { | ||
|
|
||
| START("start"), | ||
| END("end"); | ||
|
|
||
| private final String input; |
There was a problem hiding this comment.
이 Command 라는 개념을 ChessGame 의 도메인으로 볼 수 있다는 생각도 드네요.
There was a problem hiding this comment.
현재는 Command를 걷어내고, Controller에서 입력에 따른 실행 상태를 관리하는 ExecuteState로 수정하였습니다!
| private static final Map<Class, String> pieceName = new HashMap<>(Map.of( | ||
| Pawn.class, "p", | ||
| Rook.class, "r", | ||
| Knight.class, "n", | ||
| Bishop.class, "b", | ||
| Queen.class, "q", | ||
| King.class, "k" | ||
| )); | ||
| private static final String EMPTY = "."; |
| public boolean canMovePawn(final Square source, final List<Square> routes) { | ||
| if (isDiagonalUnit(source, routes.get(0))) { | ||
| return isAttackable(source, routes.get(0)); | ||
| } | ||
| return canMoveStraight(source, routes); | ||
| } | ||
|
|
||
| private boolean isDiagonalUnit(final Square source, final Square destination) { | ||
| final int distanceX = destination.calculateDistanceX(source); | ||
| final int distanceY = destination.calculateDistanceY(source); | ||
| return Math.abs(distanceX) == Math.abs(distanceY) && Math.abs(distanceX) == 1; | ||
| } | ||
|
|
||
| private boolean canMoveStraight(final Square source, final List<Square> routes) { | ||
| final Piece piece = board.get(source); | ||
| final Square destination = routes.get(0); | ||
| final int distanceY = Math.abs(destination.calculateDistanceY(source)); | ||
| return (distanceY == 2 && source.isInitPawnPosition(piece.isBlack())) | ||
| || (distanceY == 1 && !board.containsKey(destination)); | ||
| } |
There was a problem hiding this comment.
넵 저도 board -> piece -> board 이 의존관계 cycle이 우려스럽긴 합니다.
모든 구현은 trade-off 라 (역할과 책임을 한 곳에서 관리하기) vs (의존성 cycle로 부터 방어하기) 사이에서의 장단을 확인해보고 좋은 방법을 선택하면 된다고 생각합니다.
우선 저라면, Chess 말 각각이 스스로의 책임을 다하는 방식으로 (board 를 인수로 넘겨줌) 진행할 것 같습니다.
추가로 생각나는 아이디어는 boardSnapshot 정도를 board 에서 copy 한 후 넘겨주는 것도 방법이 될 수 있을 것 같아요.
| import chess.domain.board.Rank; | ||
| import java.util.function.BiFunction; | ||
|
|
||
| public enum DirectionVector { |
There was a problem hiding this comment.
Vector들은 strategy 패키지에서 관리할 수도 있을 것 같아요.
| public abstract class Piece { | ||
|
|
||
| protected final Color color; | ||
| protected final Strategy strategy; | ||
|
|
||
| protected Piece(final Color color, final Strategy strategy) { | ||
| this.color = color; | ||
| this.strategy = strategy; | ||
| } | ||
|
|
||
| public List<Square> findRoute(final Square source, final Square destination) { | ||
| return strategy.findRoute(source, destination); | ||
| } |
There was a problem hiding this comment.
route 와 관련된 로직을 관리하는 책임의 분리를 하니 깔끔해졌네요 👍
Strategy를 생성하였더니 각 기물들이 수행하는 역할이 사라지고, Strategy를 관리하는 객체가 된것 같았는데 이 부분은 괜찮은지 궁금합니다!
이 부분 관련해서 추가로 남기면, 객체 지향은 그 역할과 책임을, 알맞은 도메인 객체에 부여하는 것이라 생각해요.
어떤 방식으로 이동할지에 대한 전략을 Strategy 라는 책임에게 위임한다 정도로 보시면 될 것 같아요. 관련해서 '전략패턴'이라는 키워드를 찾아보시면 좋을 것 같아요 👍
| import java.util.ArrayList; | ||
| import java.util.List; | ||
|
|
||
| public class QueenStrategy implements Strategy { |
There was a problem hiding this comment.
Bishop, Queen, Rook Strategy를 개발하면서 느꼈을 지 모르겠지만, DirectionVector 를 사용하고 있는 Strategy의 코드들은 모두 동일해졌습니다. Knight 와 King도 마찬가지로 보이고요!
(실제 도메인 로직에서도 비슷한 동작들입니다. - 끝까지 이동 or 정해진 범위 이동)
이러한 비슷한 동작들을 공통화 해볼 수 있을 것 같아요.
| import java.util.Scanner; | ||
| import java.util.stream.Collectors; | ||
|
|
||
| public class InputView { |
|
리뷰해주신 내용을 바탕으로 다시 한번 리팩토링을 진행해 보았습니다. 가장 큰 변화는 아래와 같습니다! 1. 중복되는 한 가지 질문사항으로는 감사합니다 :) |
| final List<Square> route = piece.findRoute(source, destination); | ||
| if (!isMovable(piece, source, route)) { | ||
| if (!piece.isMovable(source, route, board.getBoardSnapShot())) { | ||
| throw new IllegalArgumentException("움직일 수 없는 위치입니다."); | ||
| } |
There was a problem hiding this comment.
piece.findRoute() 도 isMovable 내부에서 호출할 수도 있을 것 같아요!
peice가 움직일 수 있는 지 여부가 도메인 로직이 될 것 같고요!
There was a problem hiding this comment.
그렇네요 좋은 수정 포인트 알려주셔서 감사합니다 ㅎㅎ
| import java.util.Map; | ||
| import java.util.stream.Collectors; | ||
|
|
||
| public class BoardSnapShot { |
There was a problem hiding this comment.
BoardSnapshot 의 책임이 너무 많아진 것은 아닐지 고민이 되네요!
There was a problem hiding this comment.
Piece가 이동할 수 있는지 확인하기 위해 매번 SnapShot을 만들어줘야 하는데
이 부분은 저는 괜찮다고 생각합니다! JAVA의 GC를 믿습니다.. 물론 이후에 이 부분이 성능상 문제가 된다면, 최적화에 대해서 고민해볼 수 있을 것 같아요.
There was a problem hiding this comment.
작성하고보니 책임이 많아진 느낌이 들긴 했습니다 .. ! 3단계를 진행하면서 개선해보도록 하겠습니다 🔥
안녕하세요 완태 🏀 우테코 5기 백엔드 크루 다즐입니다!
이번 미션은 다른 미션에 비해 훨씬 어렵다고 생각했습니다. 그렇기에 설계 단계에서부터 꼼꼼하게 작은 부분부터 고려하려고 노력했던 것 같습니다. 미션을 시작하고 6시간 동안은 개발을 시작하지 않고 페어와 설계에 관해서만 얘기를 했던 것 같습니다.
많은 시간을 설계에 사용하다 보니 각 도메인이 가져야 하는 역할에 대해 조금 깊게 생각해볼 수 있었고, 다른 미션은 개발을 진행하며 설계가 계속 바뀌어 하나의 도메인에 집중하지 못하고 도메인을 왔다 갔다 하며 개발을 진행했던 것에 비해 이번 미션에서는 설계한 방향대로 진행한 것 같아 결과물에 대해 조금 만족하고 있습니다.
하지만 그럼에도 부족한 부분은 당연히 존재할 것으로 생각합니다. 따라서 리뷰를 통해 많이 배워가도록 하겠습니다. 사소한 부분이라도 수정이 필요해 보이거나 개선점이 보이신다면 알려주시면 감사합니다!
아래는 미션을 진행하면서 고민했던 부분에 대해서 정리해 보았습니다.
Piece 별로 다른 벡터 필드를 가지는 점에 대해
이번 미션에서
Piece는 체스 기물을 의미합니다. 체스 기물에게 시작 체스 칸과 도착 체스 칸을 전달하여 시작부터 도착까지의 경로를 반환하도록 도메인을 구현하였습니다. 움직이는 방향을 하나의 벡터라고 생각하였고, 벡터를 통해 해당 기물이 움직일 수 있는지 판단하거나 경로를 찾는 역할을 수행하도록 하였습니다.하지만
Piece별로 움직이는 방법이 다르기에 각기 다른 벡터를 가지게 되었고, 체스 기물들은Piece를 상속하지만 각각 다른 벡터 필드를 가지게 되었습니다. 이러한 방법이 괜찮은지 궁금합니다!미션을 진행하는 동안 너무 재미있었고, 리뷰도 너무너무 기대됩니다! 잘 부탁드리겠습니다 😆