Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[디디] 체스 1주차 미션 제출합니다. #52

Merged
merged 40 commits into from
Jun 14, 2020

Conversation

fucct
Copy link

@fucct fucct commented Mar 26, 2020

감사합니다. 잘부탁드립니다.

fucct added 16 commits March 24, 2020 15:07
	- Position은 내부적으로 file과 rank를 알고 있다.
	- Position을 map으로 캐싱해서 String 입력 값에 따라 해당하는 position 객체를 반환
	- Piece는 자신의 위치를 안다
	- Piece는 자신이 속한 Player를 안다
	- Board를 초기화하기 위한 BoardInitializer 인터페이스 추가
	- Board initializer가 사용할 초기 데이터인 InitialPieceRepository 추가
	- EnumRepositoryBoardInitializer에서 repository를 참고하여 board 초기화
	Todo.
	- 장애물이 있을 경우 판단하는 로직 구현 필요
	- Knight를 제외한 다른 piece의 이동 규칙 구현 필요
	- 게임의 진행 상태를 나타내는 State 생성
	- Application 로직에 있던 내용을 Controller로 이동
	- InputView의 리팩토링
	- Board 출력을 위한 ResponseDto 생성
	- OutputView에서 ResponseDto로 출력하는 로직 구현
	- Piece의 색에 따라 toString 구현
	- 한 게임을 관리하는 ChessGame 생성
	- 이동을 위한 파라미터 객체인 MoveParameter 생성
	- EnumRepository에서 생성된 피스를 가지지 않고, 피스 generator를 가지고 있도록 변경
	- EnumRespositoryInitializer에서는 해당하는 generator로 피스를 생성하여 보드 초기화
- 이동 규칙에 적합하게 움직이는 지 테스트
- 중복되는 메서드를 템플릿 메서드로 분리
- PieceState를 적절하게 반환하도록 구현
- Piece의 상태에 따라 Ready, Running, End로 분리
- Rook Test 구현
- Direction 개념 구현
- Direction 에 맞게 position 이동 구현
- Rook 이동 구현
- Bishop Test 구현
- Direction 이 음수도 나오도록 수정
- Direction 에 맞게 position 이동 수정
- Bishop 이동 구현
- King Test 구현
- Direction 에 존재하지 않는 target의 경우 예외처리
- King 이동 구현
 - 현재 Trun인 플레이어만 move가 가능하도록 구현
 - 각 Piece 이동에 맞게 테스트 refactor
 - Pawn 이동 규칙에 맞게 분리
 - 플레이어 체크 안되는 버그 수정
Copy link

@jeonghoon1107 jeonghoon1107 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요 디디
몇 가지 피드백을 남겼으니 확인해주세요

src/main/java/chess/controller/dto/RequestDto.java Outdated Show resolved Hide resolved
src/main/java/chess/ConsoleChessApplication.java Outdated Show resolved Hide resolved
src/main/java/chess/domain/MoveParameter.java Outdated Show resolved Hide resolved
src/main/java/chess/domain/MovingDirection.java Outdated Show resolved Hide resolved
src/main/java/chess/domain/ChessGame.java Outdated Show resolved Hide resolved
src/main/java/chess/domain/ChessGame.java Outdated Show resolved Hide resolved
src/main/java/chess/domain/piece/NotMovedPawn.java Outdated Show resolved Hide resolved
src/main/java/chess/domain/ChessGame.java Outdated Show resolved Hide resolved
src/main/java/chess/domain/state/EndState.java Outdated Show resolved Hide resolved
fucct added 3 commits March 30, 2020 14:26
- 접근제어자 추가
- 불필요한 객체 삭제
- 매직넘버 상수화
- 가독성이 떨어지는 메서드 분리
- indent 줄이기 위해 메서드 분리
- status 결과 객체 구현
- 결과 계산시 종료 구현
              - 컨벤션 리팩토링
              - 테스트 에러 수정
@fucct
Copy link
Author

fucct commented Mar 30, 2020

안녕하세요 리뷰어님 피드백 잘 받았습니다! 피드백 반영 도중 궁금한게 있어서 정리해서 질문드립니다! 답변주시면 정말 감사하겠습니다!!

  1. 이번에 체스 구현이 만족스럽지 못해서 개인적으로 다시 한번 구현해봤는데요, 우선 가장 마음에 걸렸던 부분이 Command가 Switch 구문을 사용해서 실행된다는 점이였습니다. 그래서 저는 Command를 enum으로 하고, UnaryOperator를 통해 start, end, status 등을 실행하도록 새로 구현해봤는데요, 여기서 문제가 move에는 파라미터가 존재해서, 다른 메서드들과 달리 파라미터를 받아야 한다는 점이였습니다. 따라서 저는 Command 에 새로운 메서드인 move(List parameters> 를 구현하고, Controller의 반복문 내에서 if구문으로 Move인 경우만 따로 처리를 해주었는데요, 이렇게 설계하는 게 다형적으로 command를 사용한 것과는 거리가 먼 것 같아서 질문드립니다. 혹시 이렇게 command간 메서드들을 통일하고 싶은데, 메서드들간 사용하는 파라미터가 다른 경우 어떤식으로 처리하는게 좋을까요? 좋은 방법이 있다면 조언해주신다면 공부해서 반영해보도록 하겠습니다.

  2. 체스 구현 도중 저희가 validateMovingPolicy를 구현하였는데요, 이 메서드는 static List 필드에 의해 동작이 변경됩니다. 또한 제가 새로 구현해본 체스게임에서는 Strategy를 사용하여 움직이지만 Strategy 내에 List을 기준으로 이동합니다. 하지만 List을 제외한 다른 부분에서 차이가 나지 않는 Piece들이 존재하는데, 만약 List 과 validateMovingPolicy의 중복을 제거하려면 어떤 방식으로 하는 게 좋을지 감이 안와서 질문드립니다. 만약 인스턴스 변수가 중복된다면 abstract class로 분리를 해줄텐데, static 한 필드가 중복되어서 어떻게 해야 할지 잘 모르겠네요😅

  3. 마지막 피드백이였던 EndState에서 Board를 갖는게 올바른지에 대해서 고민을 해보았는데요, 처음 제가 게임을 설계할 때 게임이 종료되어도 체스판을 볼 수 있고, 그에 대한 결과를 계산할 수 있어야 한다고 생각해서 Board를 갖도록 클래스를 설계했습니다. 혹시 status가 입력되거나 end가 입력되면 점수를 기준으로 승패를 따져야 하는 건가요? 리뷰어님께서 주신 피드백이 제가 생각한게 맞는지 정확하지 않아서 질문드립니다 ㅠㅠ. 일단 주신 피드백을 바탕으로 end나 status가 들어오면 종료를 하고, 그 시점의 점수를 기준으로 승패를 판단하도록 변경하였고, 그에 따라 EndState에 board가 아닌 status를 갖도록 수정했습니다. 혹시라도 제가 잘못이해한것이라면 답변주시면 반영하도록 하겠습니다 !!

바쁘신데 피드백 감사드리고 그 외에 부족한 점이나 공부해야 할 점이 있다면 같이 알려주시면 공부해서 반영하도록 하겠습니다. 감사합니다😀

Copy link

@jeonghoon1107 jeonghoon1107 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요 디디!
간단한 피드백 남겼으니 확인 부탁드려요 :)

  1. 말씀하신대로 구현되어있지 않은 것 같아요.
    enum을 key값으로 하고 행위를 value로 담아서 switch문을 없앨 수 있을 것 같아요.
    각 행위에는 정의해놓으신 RequestDto을 파라미터로 사용해도 좋을 것 같아요.

  2. MovingDirection.getDirection(position, target) 을 내부로 감추면 어떨까요?

  3. 피드백을 참고해주세요!


import java.util.function.BiFunction;

public enum InitialPieceRepository {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Repository라는 네이밍은 어울리지 않는 것 같아요.

return new EndState(createStatus());
}

public Status createStatus() {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RunningState에서 점수를 계산하는 것은 역할에 맞지 않아 보여요.
더불어 결과에 대한 테스트를 추가하면 좋을 것 같아요.


PieceDto pieceDto = boardDto.get(target);
if (ATTACK_DIRECTION_BY_PLAYER.get(player).contains(direction)) {
if (Objects.isNull(pieceDto)) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indent 규칙 위반

MovingDirection movingDirection = MovingDirection.getDirection(position, target);

if (MOVING_DIRECTION_BY_PLAYER.get(player).equals(movingDirection)) {
if (position.getRankDifference(target) != movingDirection.getRankDirection()) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indent 규칙 위반

fucct added 2 commits April 3, 2020 11:14
- state 패턴 삭제
- service layer 추가
- indent 규칙에 맞게 리팩토링
- 기타 컨벤션에 맞게 수정
fucct added 3 commits April 8, 2020 14:00
- Index page에 접근가능하도록 구현
- Index page에서 현재 존재하는 방 목록 보여주도록 구현
- DB에 접근하여 방 목록을 가져오도록 구현
- 저장시 DB에 저장
- 불러오기 시 DB 로부터 게임 load
- 진행중인 게임을 저장하지 않고 종료시 DB 삭제
- 진행중인 게임에서 새 게임 플레이 가능
Copy link

@jeonghoon1107 jeonghoon1107 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

디디!
미션 진행하느라 고생하셨어요 :)

@jeonghoon1107 jeonghoon1107 merged commit 966229b into woowacourse:fucct Jun 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants