[1,2,3단계 - 체스] 에드(박성진) 미션 제출합니다.#210
Conversation
jaeyeonling
left a comment
jaeyeonling
left a comment
There was a problem hiding this comment.
안녕하세요 에드!
체스 미션을 함께할 재연링입니다 :)
요구사항에 맞게 잘 구현해주셨네요 💯
피드백 남겨두었으니 확인해주세요!
| # java-chess | ||
| 체스 게임 구현을 위한 저장소 | ||
|
|
||
| ## 기능 목록 |
|
|
||
| import static spark.Spark.get; | ||
|
|
||
| public class WebUIChessApplication { |
There was a problem hiding this comment.
해당 서비스는 WebUI가 아닌 Console이기 때문에 클래스명을 변경하거나 새로 분리해주시면 좋을 것 같아요 :)
There was a problem hiding this comment.
해당 클래스명를 Application으로 바꾸고 Web 관련 코드 제거하겠습니다. 👍
README.md
Outdated
|
|
||
| ## 기능 목록 | ||
|
|
||
| - [ ] 체스판을 초기화한다. |
README.md
Outdated
|
|
||
| ### 질문 목록 | ||
|
|
||
| - InitBoardGenerator의 역할을 InitPieces에 통합하는게 어떤지? |
There was a problem hiding this comment.
코드를 봤을 때 자연스럽게 존재하기 때문에 유지하셔도 괜찮을 것 같고 단순한 기능을 하고 있기 때문에 통합하셔도 될 것 같네요!
취향에 맞게 선택할 수 있는 부분 같습니다 :)
|
|
||
| public class Board { | ||
| public static final int BOTH_KINGS_ALIVE = 2; | ||
| private final List<Map<Position, Piece>> squares; |
There was a problem hiding this comment.
| private final List<Map<Position, Piece>> squares; | |
| private final Map<Position, Piece> squares; |
Position은 x,y 좌표를 가지고 있기 때문에 Map으로 가능하지 않을까요!?
There was a problem hiding this comment.
이 부분은 출력 때문에 이렇게 했습니다. 이렇게 하지 않으면 매직 넘버 8을 사용해야 할 것 같아서요. Map으로 만들고 매직넘버를 쓰지 않을 수 있는 방법이 있나 찾아보겠습니다. 👍
|
|
||
| import chess.domain.board.Board; | ||
|
|
||
| public abstract class Ended extends Started { |
There was a problem hiding this comment.
해당 클래스는 어떠한 역할을 하고있을까요?
또한 Ended가 Started를 상속한다 라는 설계가 조금 어색하게 느껴지네요!
There was a problem hiding this comment.
단순히 계층 구조를 맞추기 위한 클래스입니다.
BlackTurn -> Running -> Started 처럼
End -> Ended -> Started 3층 구조를 맞출려고 했어요.
그런데 생각해보니 계층 구조를 맞출 필요가 없네요. 😅
End -> Started 로 바로 상속하도록 수정하겠습니다.
| import java.util.Map; | ||
| import java.util.stream.Collectors; | ||
|
|
||
| public class DtoAssembler { |
| @@ -0,0 +1,4 @@ | |||
| package chess.domain; | |||
|
|
|||
| public class ChessInitTest { | |||
| import static org.assertj.core.api.Assertions.assertThat; | ||
| import static org.assertj.core.api.Assertions.assertThatThrownBy; | ||
|
|
||
| public class PositionTest { |
|
|
||
| import java.util.Objects; | ||
|
|
||
| public abstract class Team { |
There was a problem hiding this comment.
Team을 Color로 대체 했는데 관련 코드를 제거 안했네요. 😅
|
재연링~!! 드디어 리펙터링이 끝났네요!!! 😊 리펙터링 과정에서 두가지 큰 변경 사항이 있었어요.
소중한 리뷰 감사합니다. 👍 |
jaeyeonling
left a comment
jaeyeonling
left a comment
There was a problem hiding this comment.
피드백 반영 잘 해주셨습니다 💯
바로 다음 단계 진행해주세요!
2 participants
안녕하세요, 재연링! 😊
체스 미션이 너무 어려워서 정신이 없네요. 😅
코드에 부족한 부분이 있다면 피드백 받으면서 열심히 고치겠습니다.
소중한 리뷰 감사합니다! 👍