[1, 2, 3단계 - 체스] 후디(조동현) 미션 제출합니다. #293
Conversation
- 테스트 코드 작성 - XAxis, YAxis 추가
- 테스트 코드 작성
- 테스트 코드 작성 - 불필요한 생성 테스트 제거
- Night 를 Knight 로 변경 - Cell 클래스 제거 - Piece 인터페이스 제거
- MoveResult 구현
| public class Board { | ||
| private final Map<Position, AbstractPiece> value; | ||
|
|
||
| private Board(Map<Position, AbstractPiece> value) { | ||
| this.value = value; | ||
| } |
There was a problem hiding this comment.
빈 칸을 표현하기 위한 '빈 말' 클래스를 piece 패키지에 구현할지, 혹은 그냥 빈칸에 대한 도메인은 구현하지 않을지 고민했어요.
위 코드와 같이 말과 말의 위치를 한쌍으로 묶기위해 Board 라는 클래스에서 Map 자료구조를 사용했는데, 현재는 '빈 말' 도메인을 구현하지 않고, 비어있는 칸은 아무 값도 넣어두지 않았습니다. 따라서 get 을 호출하면 null 이 반환되는데, 외부에서 NPE 가 발생하는 것을 방지하기 위해 게터에는 Optional 로 한번 감싸서 반환하였습니다.
src/test/java/chess/domain/game/gamestate/RunningBlackTest.java
Outdated
Show resolved
Hide resolved
- 테스트코드 작성
…ard 에서 ScoreResult 로 이동
- Score에 add, subtract 메소드 추가 - ScoreResult의 점수 계산로직 리팩토링
- OutputView가 ScoreResult를 받아 출력하도록 변경
- BoardDto 패키지 이동 (dto -> dto.board) - DTO 구현에 따른 컨트롤러, 뷰 코드 변경
- Position에 Position 두개를 전달하여 각도를 반환하는 메소드 구현 - Direction에 Position 두개를 전달하여 방향을 반환하는 메소드 구현
- DirectionStrategy 제거 - MovingStrategy에 Direction 적용
- 사용되지 않는 Position의 메소드 제거 - 폰 움직임 테스트 추가 - Board 리팩토링 - Direction에 isVertical, isHorizontal, isDiagonal 추가
- XAxis 테스트 코드 추가 - YAxis 테스트 코드 추가
|
안녕하세요, 에단! 두번째 리뷰요청이 많이 늦었죠 😓 변경 사항
개선해야할 점더 개선하다보면 피드백 텀이 너무 길어질 것 같아서 아직 아래의 내용은 개선하지 못했어요.
기타 더 궁금한 내용은 코멘트로 달아보겠습니다! 감사해요. |
| private void executeCommandIfNotEnd(ChessGame chessGame, ConsoleCommandDto commandDto) { | ||
| if (commandDto.isStart()) { | ||
| executeStart(chessGame); | ||
| } | ||
| if (commandDto.isMove()) { | ||
| executeMove(chessGame, commandDto); | ||
| } | ||
| if (commandDto.isStatus()) { | ||
| executeStatus(chessGame); | ||
| } | ||
| requestCommand(chessGame); | ||
| } |
There was a problem hiding this comment.
많은 if 문을 없애기 위해서, 컨트롤러 쪽에 ConsoleCommandDto 등을 전달받아 알맞은 명령을 실행하는 클래스를 별도로 만들어볼까 고민을 했었는데, 컨트롤러가 너무 많은 클래스를 갖게 될까봐 반영은 하지 않았어요.
아직 if 문의 반복을 줄이는 뾰족한 방법이 생각나지 않는데, 에단의 의견이 궁금합니다!
| public class PieceDto { | ||
|
|
||
| private final PieceNotation pieceNotation; | ||
| private final PieceColor pieceColor; |
There was a problem hiding this comment.
PieceDto 에서 도메인인 PieceColor 와 PieceType 을 알고있고, 사용하고 있어요. DTO 에서 도메인을 알고 있는 게 좋지 않은 것은 알고있지만, Enum 은 예외라고 생각합니다. 이 구조에 대한 에단의 의견이 궁금합니다.
| public class PieceDto { | ||
|
|
||
| private final PieceNotation pieceNotation; | ||
| private final PieceColor pieceColor; |
| private void executeCommandIfNotEnd(ChessGame chessGame, ConsoleCommandDto commandDto) { | ||
| if (commandDto.isStart()) { | ||
| executeStart(chessGame); | ||
| } | ||
| if (commandDto.isMove()) { | ||
| executeMove(chessGame, commandDto); | ||
| } | ||
| if (commandDto.isStatus()) { | ||
| executeStatus(chessGame); | ||
| } | ||
| requestCommand(chessGame); | ||
| } |
| public class Board { | ||
| private final Map<Position, AbstractPiece> value; | ||
|
|
||
| private Board(Map<Position, AbstractPiece> value) { | ||
| this.value = value; | ||
| } |
| requestCommand(chessGame); | ||
| } | ||
|
|
||
| private void requestCommand(ChessGame chessGame) { |
There was a problem hiding this comment.
requestCommand ->executeCommandIfNotEnd -> requestCommand... 처럼 메서드가 재귀적으로 실행되는데요.
requestCommand가 재귀함수지만 재귀 호출부가 다른 메서드에 있어서 메서드의 호출 흐름을 파악하기 어렵게 만드는 걸로 보여요. requestCommand의 재귀 호출은 가급적 같은 메서드에서 처리하면 어떨까요?
| return new ConsoleCommandDto(Command.from(command), arguments); | ||
| } | ||
|
|
||
| public boolean isStart() { |
안녕하세요 에단! 만나서 반갑습니다.
당연하겠지만, 지금까지의 미션 중 가장 어려운 미션이었던 것 같아요.
그럼에도 불구하고 지금까지 학습한 여러 가지를 적용해볼 수 있어서 가장 재밌었던 미션이기도 했습니다.
여러 고민사항이나 질문은 별도의 코멘트로 남길게요. 감사합니다 에단 😄
아쉬운 점
코드
특히 게터를 사용하지 않기, 한줄에 점 하나만 찍는 디미터 법칙을 지키기, 메소드 라인 제한 세가지가 미흡한 상황인 것 같습니다.
추가로 체스 말이 움직이는 다양한 경우의 수에 대해 테스트가 다소 부족하다는 생각이 들어요. 이 부분들은 두번째 피드백 요청까지 깔끔하게 해결해보겠습니다. 😓
기능 구현 목록
더하여, 기능 구현에 집중하다보니 기능 구현 목록작성이 많이 미흡해서 아쉽습니다. 페어와 함께 3단계까지 구현하고나서야 기능 구현 목록을 작성하지 않은 것을 알아챘네요. 4단계와 5단계는 신경써서 작성해보겠습니다. 🥲