-
Notifications
You must be signed in to change notification settings - Fork 0
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
[3, 4단계 - 체스] 망쵸(김주환) 미션 제출합니다. #22
base: 3juhwan
Are you sure you want to change the base?
Conversation
* docs: 기능 목록 추가 Co-authored-by: 3juhwan <[email protected]> Co-authored-by: Nam Gi Beom <[email protected]> * feat: 킹이 여덟 방향으로 움직이는지 확인하는 기능 추가 Co-authored-by: 3juhwan <[email protected]> Co-authored-by: Nam Gi Beom <[email protected]> * feat: 비숍이 대각선 방향으로 움직이는지 확인하는 기능 추가 Co-authored-by: 3juhwan <[email protected]> Co-authored-by: Nam Gi Beom <[email protected]> * test: king이 두 칸 이상 움직일 수 없는지 테스트 Co-authored-by: 3juhwan <[email protected]> Co-authored-by: Nam Gi Beom <[email protected]> * feat: 체스 말 타입 추가 Co-authored-by: 3juhwan <[email protected]> Co-authored-by: Nam Gi Beom <[email protected]> * feat: 포지션 객체 추가 Co-authored-by: 3juhwan <[email protected]> Co-authored-by: Nam Gi Beom <[email protected]> * refactor: 2단계 관련 클래스 삭제 Co-authored-by: 3juhwan <[email protected]> Co-authored-by: Nam Gi Beom <[email protected]> * feat: 체스판에 말을 놓는 기능 추가 Co-authored-by: 3juhwan <[email protected]> Co-authored-by: Nam Gi Beom <[email protected]> * feat: file 클래스 추가 Co-authored-by: 3juhwan <[email protected]> Co-authored-by: Nam Gi Beom <[email protected]> * feat: rank 클래스 추가 Co-authored-by: 3juhwan <[email protected]> Co-authored-by: Nam Gi Beom <[email protected]> * feat: 이전, 다음 file을 반환하는 기능 추가 Co-authored-by: 3juhwan <[email protected]> Co-authored-by: Nam Gi Beom <[email protected]> * feat: 움직임의 방향성을 계산하는 기능 구현 Co-authored-by: 3juhwan <[email protected]> Co-authored-by: Nam Gi Beom <[email protected]> * feat: 이동 방향을 반환하는 기능 구현 Co-authored-by: 3juhwan <[email protected]> Co-authored-by: Nam Gi Beom <[email protected]> * feat: 다음 포지션을 반환하는 기능 구현 Co-authored-by: 3juhwan <[email protected]> Co-authored-by: Nam Gi Beom <[email protected]> * feat: 피스가 움직이는 경로를 반환하는 기능 구현 Co-authored-by: 3juhwan <[email protected]> Co-authored-by: Nam Gi Beom <[email protected]> * feat: 킹, 퀸 이동 검증 구현 Co-authored-by: 3juhwan <[email protected]> Co-authored-by: Nam Gi Beom <[email protected]> * refactor: 이동 검증 클래스 추상화 Co-authored-by: 3juhwan <[email protected]> Co-authored-by: Nam Gi Beom <[email protected]> * feat: 룩 이동 검증 구현 Co-authored-by: 3juhwan <[email protected]> Co-authored-by: Nam Gi Beom <[email protected]> * feat: 비숍, 나이트 이동 검증 구현 Co-authored-by: 3juhwan <[email protected]> Co-authored-by: Nam Gi Beom <[email protected]> * style: 테스트 개행 추가 Co-authored-by: 3juhwan <[email protected]> Co-authored-by: Nam Gi Beom <[email protected]> * feat: 피스에 이동 경로 검증 기능 구현 Co-authored-by: 3juhwan <[email protected]> Co-authored-by: Nam Gi Beom <[email protected]> * refactor: 공통 로직 추상 클래스로 추상화 Co-authored-by: 3juhwan <[email protected]> Co-authored-by: Nam Gi Beom <[email protected]> * feat: 블랙 폰, 화이트 폰 추가 Co-authored-by: 3juhwan <[email protected]> Co-authored-by: Nam Gi Beom <[email protected]> * refactor: 포지션 간의 경로를 계산하는 기능을 이전 Co-authored-by: 3juhwan <[email protected]> Co-authored-by: Nam Gi Beom <[email protected]> * refactor: piece를 추상화하여 king 세부 사항 재구현 Co-authored-by: 3juhwan <[email protected]> Co-authored-by: Nam Gi Beom <[email protected]> * refactor: queen 세부 사항 재구현 Co-authored-by: 3juhwan <[email protected]> Co-authored-by: Nam Gi Beom <[email protected]> * refactor: bishop 세부 사항 재구현 Co-authored-by: 3juhwan <[email protected]> Co-authored-by: Nam Gi Beom <[email protected]> * refactor: knight 세부 사항 재구현 Co-authored-by: 3juhwan <[email protected]> Co-authored-by: Nam Gi Beom <[email protected]> * refactor: rook 세부 사항 재구현 Co-authored-by: 3juhwan <[email protected]> Co-authored-by: Nam Gi Beom <[email protected]> * feat: 중립 컬러, 컬러 확인 기능 추가 Co-authored-by: 3juhwan <[email protected]> Co-authored-by: Nam Gi Beom <[email protected]> * feat: empty 기물 추가 Co-authored-by: 3juhwan <[email protected]> Co-authored-by: Nam Gi Beom <[email protected]> * feat: pawn 세부 사항 재구현 Co-authored-by: 3juhwan <[email protected]> Co-authored-by: Nam Gi Beom <[email protected]> * feat: 초기 체스보드 생성하는 팩토리 구현 Co-authored-by: 3juhwan <[email protected]> Co-authored-by: Nam Gi Beom <[email protected]> * feat: move 커멘드 추가 Co-authored-by: 3juhwan <[email protected]> Co-authored-by: Nam Gi Beom <[email protected]> * feat: 피스를 움직이는 체스보드 구현 Co-authored-by: 3juhwan <[email protected]> Co-authored-by: Nam Gi Beom <[email protected]> * feat: 체스 보드 출력 기능 구현 Co-authored-by: 3juhwan <[email protected]> Co-authored-by: Nam Gi Beom <[email protected]> * feat: 커맨드명으로 커맨드 찾는 기능 추가 Co-authored-by: 3juhwan <[email protected]> Co-authored-by: Nam Gi Beom <[email protected]> * feat: 컨트롤러 추가 Co-authored-by: 3juhwan <[email protected]> Co-authored-by: Nam Gi Beom <[email protected]> * feat: 기물 타입 추가 Co-authored-by: 3juhwan <[email protected]> Co-authored-by: Nam Gi Beom <[email protected]> * refactor: 중복 코드 리팩토링 Co-authored-by: 3juhwan <[email protected]> Co-authored-by: Nam Gi Beom <[email protected]> * docs: 기능 목록 수정 Co-authored-by: 3juhwan <[email protected]> Co-authored-by: Nam Gi Beom <[email protected]> * feat: 두 file 사이의 file들을 찾는 기능 추가 * feat: 두 rank 사이의 rank들을 찾는 기능 추가 * refactor: 두 position 사이의 경로를 찾는 기능 분리 * refactor: 피스를 움직이는 기능 내부 메서드 분리 * test: extract 삭제 * refactor: 아군 색깔인지 판단하는 하는 기능 추상화 * refactor: 폰이 아닌 피스들에 대한 추상화, 리팩터링 * refactor: 피스 프로퍼티 메서드명 수정 * refactor: 폰을 색상에 따라 다르게 구현 * refactor: 폰 공통 로직 추상화 * style: final 추가, 예외 메시지 수정, 메서드명 수정 * style: final 추가, 메서드 순서 변경 * style: 개행 수정, 상수 분리 * refactor: 예외 처리 디테일 수정 * refactor: 경로가 막혔는지 확인하는 기능을 Route 클래스로 분리 * refactor: 커맨트 패턴 적용 * refactor: view, controller에 커맨드 적용, 예외가 발생해도 종료되지 않게 수정 * test: 예외 메시지 수정 * feat: 턴 개념 추가 * style: 테스트 변수명 수정 resource -> source * style: 폰 테스트를 색상에 따라 분리 * feat: 게임에 턴 적용 * refactor: MessageResolver 추가 * feat: 각 명령어 별 유효성 검증 추가 * fix: 출발 지점에 피스가 존재하지 않는 경우에 예외 발생 * fix: 8번째 rank가 출력되지 않는 버그 수정 * style: 폰 움직임 거리에 대하 상수명 수정 * style: 예외 메시지 수정 * style: 사용하지 않는 상수 제거 * docs: 추가 룰에 대한 문서 수정, 구현한 사항 체크 * style: 누락된 final 키워드 추가 * style: 필드 변수를 소문자로 수정 * refactor: 턴을 ChessBoard가 직접 관리하게 수정 * test: 개행 추가, 빈 리스트 검증에 isEmpty() 메서드 사용 --------- Co-authored-by: Nam Gi Beom <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi 망쵸! 전반적으로 깔끔한 코드 잘 봤어!
솔직한 감상 평으로는 '내가 감히 범접할 수 없는 깔끔한 코드...'
따로 추가적으로 할 말은 거이 없었고, 사소한 부분들을 짚어봤어
남은 레벨1도 화이팅!
private void validateArgumentSize(final List<String> arguments) { | ||
if (arguments.size() != ARGUMENT_SIZE) { | ||
throw new IllegalArgumentException(); | ||
} | ||
} | ||
|
||
private void validateRoomIdFormat(final String input) { | ||
try { | ||
Integer.parseInt(input); | ||
} catch (NumberFormatException e) { | ||
throw new IllegalArgumentException(); | ||
} | ||
} | ||
|
||
private void validateRoomIdRunning(final List<RoomDto> validRooms, final String input) { | ||
boolean isRunningRoomNotFound = validRooms.stream() | ||
.noneMatch(room -> room.room_id() == Integer.parseInt(input)); | ||
if (isRunningRoomNotFound) { | ||
throw new IllegalArgumentException(); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
예외 메시지 일부러 안 넣으신거 아니죠?
public void run() { | ||
UserDto user = userController.run(); | ||
while (true) { | ||
RoomDto room = gameRoomController.run(user); | ||
chessGameController.run(room); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
개인적으로 가장 마음에 드는 부분!
여러 컨트롤러를 이쁘게 사용한 느낌?
이렇게 보다 보니까, 패키지를 controller, service, dao, domain로 구분하는게 아니라
user, gameroom, chessgame으로 나눈 다음에 각각 하위에 controller, service, dao, domain이 있으면 좋을 것 같은 느낌? (전체 구조는 잘 몰라서 추천 정도만...)
public Connection getConnection() { | ||
try { | ||
return DriverManager.getConnection("jdbc:mysql://" + SERVER + "/" + DATABASE + OPTION, USERNAME, PASSWORD); | ||
} catch (final SQLException e) { | ||
System.err.println("DB 연결 오류:" + e.getMessage()); | ||
e.printStackTrace(); | ||
return null; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
null을 반환해주는 이유가 있어?
나는 정상 실행이 불가능하다고 생각해서 IllegalStateException
을 던지는뎁
private void setParameters(final PreparedStatement preparedStatement, final String... parameters) throws SQLException { | ||
for (int i = 0; i < parameters.length; i++) { | ||
preparedStatement.setString(i + 1, parameters[i]); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이건 어때?
private void setParameters(final PreparedStatement preparedStatement, final String... parameters) throws SQLException { | |
for (int i = 0; i < parameters.length; i++) { | |
preparedStatement.setString(i + 1, parameters[i]); | |
} | |
} | |
private void setParameters(final PreparedStatement preparedStatement, final String... parameters) throws SQLException { | |
for (int i = 1; i <= parameters.length; i++) { | |
preparedStatement.setString(i, parameters[i]); | |
} | |
} |
public void execute(final String query, final String... parameters) { | ||
try (final Connection connection = connectionManager.getConnection(); | ||
final PreparedStatement preparedStatement = connection.prepareStatement(query)) { | ||
setParameters(preparedStatement, parameters); | ||
preparedStatement.executeUpdate(); | ||
} catch (final SQLException e) { | ||
throw new IllegalArgumentException(e.getMessage()); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아니... JDBC 템플릿을 만들었네 ㅎㄷㄷ...
덕분에 DAO가 보기 좋아진 것 같아!
public interface PieceDao { | ||
void add(RoomDto room, PieceDto piece); | ||
|
||
List<PieceDto> findPieceByGameId(int gameId); | ||
|
||
void deleteAllByGameId(int gameId); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PieceDao를 인터페이스로 분리한 이유가 있을까요?
하나의 클래스만 있으면 되는데, 인터페이스가 있는 건 과한 설계 아닐까요?
src/main/java/view/InputView.java
Outdated
public static controller.game.command.Command readGameCommand() { | ||
try { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
??? controller.game.command.Command
아마도 패키지 옮기면서 수정이 안된 듯...
PAWN(1.0, (color) -> { | ||
if (color.isBlack()) { | ||
return new BlackPawn(); | ||
} | ||
return new WhitePawn(); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PAWN(1.0, this::createPawn)
...
private Piece createPawn(Color color) {
if (color.isBlack()) {
return new BlackPawn();
}
return new WhitePawn();
}
src/main/resources/sample.txt
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 파일을 굳이 올린 이유가 따로 있어?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
망쵸! 코드 잘 읽었어요! 전체적으로 잘 작성해주셔서 리뷰 할 내용이 많이 없네요! 고생하셨어용
try { | ||
Integer.parseInt(input); | ||
} catch (NumberFormatException e) { | ||
throw new IllegalArgumentException(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
예외 메시지가 존재하지 않는데 의도하신걸까요..! 🤔
import dto.UserDto; | ||
import service.GameRoomService; | ||
|
||
public interface Command { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Command
파라미터로 DTO
를 받고 있는데 만약 DTO에 변경이 생긴다면 Command
까지 함께 변경될 여지가 생길거 같아요..!
} catch (final SQLException e) { | ||
System.err.println("DB 연결 오류:" + e.getMessage()); | ||
e.printStackTrace(); | ||
return null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
커넥션 생성을 실패하면 null
을 반환하도록 로직을 작성해주셨군요..! 지금의 서비스에서는 DB
접속에 문제가 생긴다면 대부분의 서비스를 활용하지 못하는 상태이니 차라리 바로 예외를 던지면 어떨까요..!
public interface GameStateDao { | ||
void add(StateDto stateDto); | ||
|
||
Optional<StateDto> findByGameId(int gameId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Optional
사용 👍 하지만 DAO
의 DB조회 메서드 결과가 DTO
로 설정되어 있어서 DTO
의 변경에 DAO로직까지 크게 영향을 받을거 같은 느낌이에요..! 아무래도 DTO
는 데이터 전송 객체인 만큼 변경 가능성이 더 잦기 때문에 Entity
를 도입해보는건 어떨까요? 🤭
public interface PieceDao { | ||
void add(RoomDto room, PieceDto piece); | ||
|
||
List<PieceDto> findPieceByGameId(int gameId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
위 코멘트와 같은 의견입니다..!
jdbcTemplate.execute(insertQuery, userDto.username(), "" + roomDto.room_id()); | ||
} | ||
|
||
public Optional<RoomDto> addNewRoom(final UserDto userDto) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
조회 메서드와 마찬가지로 저장 로직 역시 Entity
를 도입해보면 어떨까요!
고생하셨습니다..!