[럿고] 체스 1주차 미션 제출합니다. #96
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
체스 1주차 머지 요청합니다.
hongsii
suggested changes
Mar 28, 2020
Comment on lines
34
to
35
| && !source.isSamePlayer(target) | ||
| && validatePawnException(source, target, Direction.getDirection(from, to))) { |
There was a problem hiding this comment.
Pawn이 아니라도 모든 체스 기물은 자신의 플레이어의 체스 기물로 이동할 수 없지 않나요?
* refactor : 입력 로직에 대한 리팩토링 * refactor : controller while문을 do while로 변경 * refactor : Custom Exception 생성 및 초기 가이드 출력 리팩토링 * refactor : isSamePlayer 메소드 if문 return으로 리팩토링 * refactor : 나이트 이동 방향 내부 클래스 리팩토링 * refactor : 폰 전진 기능 리팩토링 * reafactor : Direction에 있는 PositionsBetween 중복코드 제거 리팩토링 Co-authored-by: Seyun <ksy90101@gmail.com>
- Optional 잘못 사용한 부분 수정 - 팩토리 수정 - enum 비교 수정
hongsii
reviewed
Mar 31, 2020
Comment on lines
+40
to
+42
| if (target instanceof King) { | ||
| this.isKingTaken = true; | ||
| } |
There was a problem hiding this comment.
move(...) 메서드에서 호출하는 것은 전부 다른 메서드이지만, 여기만 메서드 레벨의 스코프가 아니기 때문에 추출해서 들여쓰기를 제거해보면 좋을 것 같습니다!
Comment on lines
34
to
35
| && !source.isSamePlayer(target) | ||
| && validatePawnException(source, target, Direction.getDirection(from, to))) { |
hongsii
approved these changes
Mar 31, 2020
| if (Objects.nonNull(target) && source.isSamePlayer(target)) { | ||
| throw new NotMoveException("같은 Player의 기물로는 이동할 수 없습니다."); | ||
| } | ||
| ; |
| public int getPawnCountPerStage(List<Piece> columnLine, Player player) { | ||
| return (int) columnLine.stream() | ||
| .filter(piece -> piece instanceof Pawn) | ||
| .map(piece -> (Pawn) piece) |
There was a problem hiding this comment.
Pawn 이 아니여도 플레이어를 가져올 수 있으니, 캐스팅이 필요 없지 않을까요?
| QUEEN("Q", 9, Row.values().length, Column.values().length), | ||
| BISHOP("B", 3, Row.values().length, Column.values().length), | ||
| ROOK("R", 5, Row.values().length, Column.values().length), | ||
| KNIGHT("N", 2.5,2, 2), |
| Row biggerRow = Row.getBigger(from.getRow(), to.getRow()); | ||
|
|
||
| List<Row> rows = Arrays.asList(Row.values()) | ||
| .subList(smallerRow.ordinal() + 1, biggerRow.ordinal()); |
There was a problem hiding this comment.
해당 로직은 여러 군데에서 사용되는 것 같은데, smallerRow.between(biggerRow) 같은 메서드를 만들어 보면 어떨까요?
로직을 한 곳에서 관리한다면 혹시나 일어날 변경 시 유지보수가 좀 더 쉬워질 것 같습니다.
| public static String key(Row row, Column column) { | ||
| Objects.requireNonNull(row); | ||
| Objects.requireNonNull(column); | ||
| StringBuilder PositionKey = new StringBuilder(); |
Comment on lines
+33
to
+36
| piece.ifPresent(value -> stringBuilder.append(value.getDisplay())); | ||
| if (!piece.isPresent()) { | ||
| stringBuilder.append(EMPTY); | ||
| } |
There was a problem hiding this comment.
아래와 같이 바꿔도 좋을 것 같아요~
String display = Optional.ofNullable(chessBoard.get(Positions.of(row, column)))
.map(Piece::getDisplay)
.orElse(EMPTY);
stringBuilder.append(display);
kingbbode
pushed a commit
that referenced
this pull request
Apr 17, 2020
* [럿고] 체스 1주차 미션 제출합니다. (#96) * docs: 1단계 요구사항 작성 * feat: 위치 정보 생성 * feat: 체스 말 생성 * feat: 체스판 초기화 및 테스트 * feat: 명령어 입력 기능 구현 * feat: 체스판 출력 기능 구현 * refactor: 2중 for문 stream으로 수정 * docs: Level2 요구사항 추가 * feat: 기물이 움직이는 방법을 나타내는 MoveRule Interface 구현 * feat: 각 기물에 맞는 움직임 상태 추가 * feat: move 명령어를 입력했을 때, King 이동 가능한 범위인지 확인 * feat: 사용자 추가, 체스 기물 분리 * feat: 이동 시 장애물 처리를 위한 이동 경로 확인 * refactor : 이동 경로 장애물 확인 수정 * feat: Knight 이동 구현 * faet : Pawn 이동 기능 구현 * refactor: Pawn 예외 처리 * refactor: Pawn 예외 추가 처리 및 전반적인 수정 * test : 킹 기물에 대한 테스트 수정 및 테스트 추가, empty 테스트 추가 * test : 체스 기물에 대한 테스트 케이스 추가 * test: Chess Board 리팩토링 * docs: 3단계 요구 사항 추가 * feat: 체스 점수 계산 구현 * refactor: 불필요한 import 제거 * feat : Pawn 점수 계산 예외 처리 * refactor: 상태를 저장하는 Status 생성 * feat : 승패결과를 구하는 기능 구현 * feat : status 명령 시, 각 플레이어의 점수와 승패 결과를 출력 하는 기능 구현 * 리팩토링한 부분이에요! (#2) * refactor : 입력 로직에 대한 리팩토링 * refactor : controller while문을 do while로 변경 * refactor : Custom Exception 생성 및 초기 가이드 출력 리팩토링 * refactor : isSamePlayer 메소드 if문 return으로 리팩토링 * refactor : 나이트 이동 방향 내부 클래스 리팩토링 * refactor : 폰 전진 기능 리팩토링 * reafactor : Direction에 있는 PositionsBetween 중복코드 제거 리팩토링 Co-authored-by: Seyun <ksy90101@gmail.com> * refacotr: parameter null 체크 추가 * feat: 무승부일 때 추가 * feat: King이 잡혔을 때 게임 종료 * refactor: Column, Row 비교 수정 * test: Display Name 수정 * refactor: Pawn 이동 오류 수정 * refactor: Piece 및 move 검증하는 구조 수정 * refactor : Direction 리팩토링 * refactor : validateTileSize 중복 코드 제거 * refactor: Empty 제거 및 Chess Board Piece 생성 방식 수정 * refactor: 피드백 반영 - Optional 잘못 사용한 부분 수정 - 팩토리 수정 - enum 비교 수정 * refactor: Optional 수정 * refactor: Input 받는 부분 수정 * refactor: exception 분리 * style: formatting * refactor: Controller 구조 수정 Co-authored-by: moonyoungCHAE <xpf_fl@naver.com> Co-authored-by: Seyun <ksy90101@gmail.com> * feat: 체스 보드 초기화 * refactor : conflict 수정 * feat: 체스 보드 기물 초기화 * feat: 체스 기물 이동 (drag and drop) * refactor : piece 중복 제거 * feat: 체스 기물 이동을 Server와 연동, Obstacle 검증 수정 * feat: 웹 페이지에서 점수 출력 * feat: turn 예외 처리 * refactor: Exception 구체화 * refactor: pawn, knight 오류 수정 * feat: 왕이 잡혔을 때 0점 처리 * refactor : Dao 생성 및 초기 게임 DB 저장 기능 구현 * refactor : 움직임에 맞춰 체스보드 저장 * feat : 게임이 끝나지 않은 체스보드 DB에서 불러오는 기능 구현 * feat: King 잡았을 때 점수 오류 수정 * feat: turn, score css 수정 * feat : 상태를 줘서, 체스보드를 새로 만들지 원래 체스보드를 가져올 지 기능 구현 * feat : 게임이 진행되는지 끝나는지에 따라 체스보드 생성 기능 추가 * refactor: Custom Exception 제거 * feat: 게임 상태 관리 추가 * feat: 움직이지 않는 경우 예외 처리 * feat: 알림창 css 수정 * refactor : dao 분리 작업 * refactor : 웹 view 수정 * refactor: 기능 오류, 구조 수정 * refactor: JDBC 연결 try with resource로 수정 Co-authored-by: SeYun <53366407+ksy90101@users.noreply.github.com> Co-authored-by: Seyun <ksy90101@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
안녕하세요. 이번에 리뷰를 받게 된 럿고입니다!
열심히 했는데, 아직 구현하지 못한 기능도 있고, 부족한 코드가 있는 것 같습니다!
피드백 주시면 열심히 배워가겠습니다! 감사합니다!