[1단계 - 보드 초기화, 기물 이동] 윌슨(박형균) 미션 제출합니다. #40
Conversation
|
|
||
| import static org.assertj.core.api.Assertions.assertThat; | ||
|
|
||
| public class PathTest { |
There was a problem hiding this comment.
PathTest와 PawnTest의 패키지 경로가 잘못 지정된 것 같아요. 👀
|
|
||
| protected abstract List<Path> getCoordinatePaths(); | ||
|
|
||
| protected abstract List<ChessPosition> getCoordinateDestinations(List<Path> coordinates, ChessPiecePositions positions); |
There was a problem hiding this comment.
현재 각 기물(JanggiChessPiece)들이 본인의 이동 경로를 탐색하기 위해상위 객체인 ChessPiecePositions를 매개변수로 받고 사용하고 있어요. 이 경우에는 아래와 같은 장단점이 있어요
...
상위 객체라는 표현에 조금 혼동이 오는데요. 👀 (상위 타입을 말씀하시는건가했어요. 🙄)
먼저 상위 객체라고 표현하신 이유를 말씀해주시면 좋을 것 같아요. 😃
현재 getDestinations를 실제로 호출하고있는 부분은 없다보니 조금 더 실질적인 구현단을 보면
좋을 것 같은데, 아쉽게도 아직은 호출하고있는 곳이 없어보이네요. 🥲
실제로 장기판을 초기화하고 각 기물들을 턴에따라 이동시킬 수 있도록 만들어보면 어떨까요?
기물을 이동시킬 수 있는 콘솔 애플리케이션을 만들어보면 좋을 것 같아요. 😉
현재 보드 초기화 << 를 한 보드판이 따로 없는 것 같은데요.
저는 개인적인 생각에서는 ChessPiecePositions이 실질적인 보드판의 역할을 하기에 딱 좋아보인다고 생각이들어요.
저는 체스가 특정 칸으로 이동할 수 있는지 없는지 여부는 보드판의 역할이라고 생각해요.
저는 JanggiChessPiece가 현재 위치에서 자신이 갈 수 있는 모든 경로를 제공해주면
보드판이 현재 기물의 상황을 고려하여 이동할 수 있는 선택지를 추려서 보여주고 선택한 위치로 ChessPiece를 이동하라고 메시지를 보내주면 된다고 생각해요.
단순히 JanggiChessPiece의 기준에서는 특정 칸으로 이동하라고했을 때, 적절히 자신의 역할을 수행하면 될 것 같다고 생각해요.
| protected abstract List<Path> getCoordinatePaths(); | ||
|
|
||
| protected abstract List<ChessPosition> getCoordinateDestinations(List<Path> coordinates, ChessPiecePositions positions); |
There was a problem hiding this comment.
이 둘은 JanggiChessPiece 클래스 내부에서만 사용되는 것 같은데, 접근제한자를 protected로 두신 이유가 있으신가요? 👀
| @Override | ||
| protected abstract List<ChessPosition> getCoordinateDestinations(List<Path> coordinates, ChessPiecePositions chessPiecePositions); |
There was a problem hiding this comment.
이미 상위 추상 클래스나 인터페이스에서 추상 메서드를 정의하고있다면, 별도로 선언하지 않아도 될 것 같아요.
UnlimitedMoveChessPiece, LimitedChessPiece , JanggiChessPiece, ChessPiece 모두 확인해서 하위타입에 불필요한 추상메서드 정의가 있다면 제거해주세요. 🙏
| @Override | |
| protected abstract List<ChessPosition> getCoordinateDestinations(List<Path> coordinates, ChessPiecePositions chessPiecePositions); |
| final List<Path> paths = new ArrayList<>(); | ||
| for (Direction direction : directions) { | ||
| final List<ChessPosition> chessPositions = new ArrayList<>(); | ||
| for (int distance = 1; distance <= 9; distance++) { |
There was a problem hiding this comment.
1, 9는 이 코드를 처음보는 사람의 입장에서 어떤 의미를 가지는지 알기가 어려울 것 같아요. 😢
적절히 상수로 추출하여 네이밍으로 의도를 드러내보면 어떨까요? 🤗
|
|
||
| import static domain.Direction.*; | ||
|
|
||
| public class King extends LimitedChessPiece { |
There was a problem hiding this comment.
궁이나 궁성은 아직 구현하지 않아도 되니, 아쉽지만 King과 Guard는 잠시 제거해주시면 좋을 것 같아요. 🙏
2.1단계 미션 진행하면서 다시 부활시켜보시죠. 😉
| import static domain.Direction.RIGHT; | ||
|
|
||
| public class Cannon extends UnlimitedMoveChessPiece { | ||
| private static final List<Direction> directions = List.of(UP, DOWN, LEFT, RIGHT); |
| import java.util.ArrayList; | ||
| import java.util.List; | ||
|
|
||
| public abstract class LimitedChessPiece extends JanggiChessPiece { |
There was a problem hiding this comment.
LimitedChessPiece는 LimitedMoveChessPiece 를 의미하시는걸까요..?
혹시 깜빡하고 Move가 빠진거라면 조금 더 명확한 의미 전달을 위해서 추가해주시면 좋을 것 같아요. 😃
LimitedChessPiece 와 UnlimitedMoveChessPiece 는 어떤 차이가있나요?
언뜻 보기에는 움직임에 제한이 있냐 없냐의 차이인 것 같은데 의미를 바로 이해하기가 조금 어려운 것 같네요.
조금 더 추가적인 설명을해주시면 좋을 것 같아요.
제가 잘 이해하지 못해서 그런걸 수도 있는데, UnlimitedMoveChessPiece, LimitedChessPiece의 계층을 두신 이유가 궁금해요. 🤔
| ### 보드 | ||
|
|
||
| - [ ] 특정 위치에서 이동 가능한 위치 리스트를 반환한다 | ||
| - [ ] 점수를 계산 한다 | ||
| - [ ] 예외 처리 | ||
| - [ ] 해당 위치에 기물이 없으면 예외 |
| @@ -1,3 +1,51 @@ | |||
| # java-janggi | |||
|
|
|||
| 장기 미션 저장소 | |||
|
|
|||
| ## 미션을 수행하며 반드시 고민할 것 | |||
There was a problem hiding this comment.
프롤로그 작성이 누락된 것 같아요. 🥲
아무래도 첫 리뷰 요청 마감 기간이 있다보니 시간이 조금 빠듯하셔서 프롤로그까지 작성하시기가 어려우셨을 것 같아요.
다음 리뷰 요청 전에는 프롤로그 작성도 같이 부탁드릴게요. 🙏😉
체크 리스트
test를 실행했을 때, 모든 테스트가 정상적으로 통과했나요?객체지향 생활체조 요구사항을 얼마나 잘 충족했다고 생각하시나요?
1~5점 중에서 선택해주세요.
선택한 점수의 이유를 적어주세요.
기물에 관련된 기능들이 계층으로 잘 나누어져 있다.
어떤 부분에 집중하여 리뷰해야 할까요?
✅ 하위 객체에서 상위 객체를 참조해도 되는가??
현재 각 기물(
JanggiChessPiece)들이 본인의 이동 경로를 탐색하기 위해상위 객체인ChessPiecePositions를 매개변수로 받고 사용하고 있어요. 이 경우에는 아래와 같은 장단점이 있어요결과적으로 이처럼 장점과 단점이 공존하는 트레이드 오프 상황이라 현재 구조가 객체지향적인지 판단이 명확하게 되지 않는 상황인데요. 이에 대해서 로키의 생각은 어떠신지 궁금해요