폴로 체스 #1
폴로 체스 #1
Conversation
* docs: 기능 구현목록 작성 * feat: 기물 종류 구현 * feat: square 구현 * test: 기물이 초기 위치에 있는지 확인하는 테스트코드 작성 * feat: initialize를 통해 기물들을 초기 위치에 위치시키는 기능 * refa: convention * feat: 체스보드를 출력하는 기능 구현 * test: square의 좌표를 반환하는 테스트코드 작성 * feat: square의 좌표를 반환하는 기능 구현 * test: 좌표로 square를 반환하는 기능 구현 테스트코드 작성 * feat: 좌표로 square를 반환하는 기능 구현 * test: king이 움직일 수 있는 좌표를 반환하는 기능에 대한 테스트 코드 작성 * feat: king이 움직일 수 있는 좌표를 반환하는 기능 구현 * feat: 방향에 대한 enum 구현 * feat: 기물이 움직일 수 있는 좌표를 반환하는 추상 메서드 구현 * feat: 중복인 경우를 제외하기 위해 Set으로 선언 * test: bishop이 움직일 수 있는 좌표를 반환하는 기능에 대한 테스트 코드 작성 * feat: bishop이 움직일 수 있는 좌표를 반환하는 기능 구현 * test: rook이 움직일 수 있는 좌표를 반환하는 기능에 대한 테스트 코드 작성 * feat: rook이 움직일 수 있는 좌표를 반환하는 기능 구현 * feat: 좌표로 square를 반환하는 생성자 오버로딩 * feat: 반환 타입 수정 * fix: king 이동 버그 디버깅 * test: knight가 이동할 수 있는 square를 반환하는 기능에 대한 테스트코드 작성 * feat: knight가 이동할 수 있는 square를 반환하는 기능 구현 * feat: 폰의 이동경로를 반환하는 기능 구현 * refa: 체스판 이동 범위 확인하는 메서드 분리 * refa: 중복되는 메서드 재사용 * refa: method이름 변경 * feat: queen의 이동경로를 반환하는 기능 구현 * feat: 이동 경로에 있는 기물들의 진영정보와 targetSquare를 받아 이동가능지 확인하는 기능 구현 pawn을 제외한 모든 기물들이 공통으로 사용할 수 있다. * refa: canMove 메소드 분리 * feat: pawn이 경로의 진영정보와 targetSqaure를 전달받아 이동가능 한지 확인하는 기능 구현 * refa: fetchMovePath method 분리 * refa: empty 클래스 싱글톤 인스턴스 반환하도록 변경 * feat: String으로 해당 File과 Rank를 찾는 기능 구현 * feat: 기물을 움직이는 기능 구현 * feat: 게임 시작을 알리는 메세지 출력 기능 구현 * feat: 명령을 입력받는 기능 구현 * refa: 움직일 수 없는 경로에 대한 예외 처리 기능 구현 * refa: 예외 메세지 작성 * refa: 컨벤션에 맞게 정리 및 메서드 분리 * feat: 체스 게임 실행 기능 구현 * feat: 에러 발생 시 재입력 받는 기능 구현 * feat: 에러 메세지 출력 기능 구현 * feat: 턴에 맞는 진영의 기물만 움직이도록 하는 기능 구현 * test: 잘못된 명령어에 대한 테스트코드 작성 * test: 잘못된 targetSquare 대한 테스트코드 작성 * fix: Command.End value 수정 * refa: isInRange 논리연산자 수정 * refa: 비숍 움직이는 경로 반환하는 로직 수정 * refa: 상속을 통한 중복되는 코드 제거 * refa: king 상속을 통한 중복되는 코드 제거 * refa: knight 중복코드 제거 * fix: distance 계산하는 로직 수정 * test: rook 마이너스 방향 이동 테스트 추가 * refa: pawn 이동 로직 리팩토링 * refa: Piece 필요없는 메소드 삭제 * refa: Piece를 상속 받는 추상클래스 UnrestrictedPiece와 RestrictedPiece 생성 * refa: 코딩컨벤션 * refa: 사용하지 않는 클래스 삭제 * fix: black pawn 더블스텝 구현안되던 부분 해결 * fix: pawn 대각선 이동로직 수정 * refa: square 캐시기능 구현 * docs: 기능 구현 목록 업데이트 * refa: pawn 더블스텝 validate 로직 변경 flag를 이용하던 방식에서 초기 랭크에 따른 더블스텝 가능여부 확인하는 방식으로 변경 * refa: Piece를 에 Type을 갖고 있게끔 변경 * refa: controller Action을 통해 리팩토링
PieceType 에서 PieceFactory로 이동
There was a problem hiding this comment.
안녕하세유 폴로 ㅎㅎㅎ 코드 잘 읽었습니다 :) 피스들을 restricted, unrestricted로 구분하고 패키지까지 구분하여 나타내주셔서 가독성이 올라갔다고 느꼈습니다! 블랙잭에서도 그렇고 패키지를 정말 잘 나누시는 것 같아요!! 룸에 대한 구현도 해주셔서 오~ 이렇게 룸을 구현할 수 있겠구나 느꼈네요. 저는 하지 못했거든요...ㅜㅜ
이번에도 이것저것 제가 알고 있는 컨벤션에 대해서 코멘트 남겨보았고, 폴로의 생각이 궁금한 것도 몇 가지 질문하였습니다. 방학동안 고생 많으셨습니다! 내일 뵙겠습니다 :)
|
|
||
| public static GameRoomCommand find(String command) { | ||
| return Arrays.stream(GameRoomCommand.values()) | ||
| .filter(gameRoomCommand -> !gameRoomCommand.equals(NONE)) |
| if (bytesLength > 10) { | ||
| throw new IllegalArgumentException("방 이름은 한글은 최대 5글자, 영어는 최대 10글자 까지 가능합니다."); | ||
| } |
There was a problem hiding this comment.
서비스에 검증 로직이 있는 것보다, 컨트롤러에 검증 로직이 있는 것이 더 합당해보이는데 폴로는 어떻게 생각하시나요!?
더불어 폴로가 서비스 계층을 도입하게된 계기가 궁금합니다. 서비스 계층에 비즈니스 로직이 있어도 된다고 생각하시나요!? 아니면 비즈니스 로직은 없고 전체적인 로직의 흐름 순서를 보장하며, dao에 접근하는 기능만 있어야 한다고 보시나요!?!
| Square currentSquare = getCurrentSquare(currentSquareInput); | ||
| Square targetSquare = getTargetSquare(targetSquareInput); |
| public void initialize() { | ||
| Rank WHITE_WITHOUT_PAWN_RANK = Rank.ONE; | ||
| Rank BLACK_WITHOUT_PAWN_RANK = Rank.EIGHT; | ||
| Rank WHITE_PAWN_RANK = Rank.TWO; | ||
| Rank BLACK_PAWN_RANK = Rank.SEVEN; | ||
|
|
||
| initializeEmpty(); | ||
| initializePawns(WHITE_PAWN_RANK, BLACK_PAWN_RANK); | ||
| initializeKings(WHITE_WITHOUT_PAWN_RANK, BLACK_WITHOUT_PAWN_RANK); | ||
| initializeQueens(WHITE_WITHOUT_PAWN_RANK, BLACK_WITHOUT_PAWN_RANK); | ||
| initializeBishops(WHITE_WITHOUT_PAWN_RANK, BLACK_WITHOUT_PAWN_RANK); | ||
| initializeKnights(WHITE_WITHOUT_PAWN_RANK, BLACK_WITHOUT_PAWN_RANK); | ||
| initializeRooks(WHITE_WITHOUT_PAWN_RANK, BLACK_WITHOUT_PAWN_RANK); | ||
| } |
There was a problem hiding this comment.
팩토리 객체를 하나 두어, 외부에서 초기화된 Map을 넘겨주는 방식으로 결합도를 줄일 수 있을 것 같네요 ㅎㅎㅎ
테스트와 확장성에 이점이 있을 것 같습니다.
| private static class singleEmptyInstanceHolder { | ||
| public static final Empty INSTANCE = new Empty(); | ||
| } |
There was a problem hiding this comment.
하지만 이곳에서 사용되는 기물들이 애플리케이션 실행 중에 무조건 실행되는 것이면 레이지 초기화를 지양해야 한다고 생각합니다.
지연 초기화는 JVM에서 실행될 때에는 아무 행동을 하지 않다가 사용자가 이쪽으로 접근을 하면 그제서야 메모리에 로딩된다고 알고 있습니다. 만약 이 기물의 생성 비용이 크다면 사용자가 애플리케이션 실행 도중에 딜레이가 생길 수 있는데 이는 좋지 않다고 생각합니다. 차라리 초기에 애플리케이션을 실행 했을 때 오랜 시간이 걸리게 하고, 게임 실행 중에는 딜레이를 없애도록 하는 것이 좋아보입니다. 폴로는 어떻게 생각하시나요!?
| protected Integer calculateDistance(List<Integer> gaps) { | ||
| List<Integer> absGaps = gaps.stream() |
There was a problem hiding this comment.
추상 클래스에서 메서드 바디를 제공하는 경우라면, 하위에서 재정의하지 못하도록 final 키워드를 메서드 레벨에 붙이는 것은 어떨까요!? 하위에서 실수로 재정의하여 의도와는 다른 동작을 하는 실수를 줄일 수 있을 것 같습니다.
|
|
||
| public List<Integer> toCoordinate() { | ||
| ArrayList<Integer> coordinate = new ArrayList<>(); | ||
| coordinate.add(file.ordinal()); |
There was a problem hiding this comment.
Enum에서 기본적으로 제공하는 ordinal 메서드는 자료구조 등에서 내부적으로 해싱처리를 위해서만 사용하는 것이 좋다고 알고 있습니다. EnumMap에서 내부적으로는 ordinal을 사용해 Key를 매핑하는 것이 사용 예시 중 하나인 것 같네요!
아마 Enum 요소의 순서가 바뀐다면 의도한대로 동작하지 않기 때문에, ordinal을 사용하지 말라는 것 같습니다 ㅎㅎㅎ
File의 멤버 변수로 int 변수를 가지도록하여 이를 비교하는 것이 더 안전할 것 같습니다!
| @FunctionalInterface | ||
| public interface PieceGenerator { | ||
| Piece apply(Camp camp); | ||
| } |
There was a problem hiding this comment.
So coool! 혹시 Function 함수형 인터페이스와 스펙이 같은데도 불구하고 재정의해주신 이유가 있을까요!?
| @@ -0,0 +1,172 @@ | |||
| package repository.game; | |||
There was a problem hiding this comment.
repository라는 패키지명은 레포지토리 계층을 나타내기 위해서 사용하신 것일까요!?
| import domain.piece.Piece; | ||
| import dto.ScoreDto; | ||
|
|
||
| public class OutputView { |
There was a problem hiding this comment.
모든 메서드가 static이네요! static을 사용하여서 구성하신 이유가 궁금합니다 ㅎㅎㅎ
MVC 패턴은 도메인과 뷰가 서로 참조하지 않고 독립적으로 개발하기 위함인데, OuputView의 메서드가 모두 static이라면 도메인에서 OuputView를 무분별하게 호출할 수 있지 않을까요!? 도메인에서 그저 호출하지 않으면 괜찮은 부분일까요!?
이부분에 대해서 저도 고민을 했던 적이 있어서 폴로와 의견을 나눠보고 싶네요 ㅎㅎㅎ
체스 재구현 할 자신도 없고...
다른 거 때문에 시간도 안될 것 같아서,
기존에 했던 코드입니다.ㅎㅎ