[3, 4단계 - 체스] 다즐(최우창) 미션 제출합니다. #548
Conversation
| public interface ChessGameCommand { | ||
|
|
||
| ChessGame execute( | ||
| final ChessGameService chessGameService, | ||
| final PieceService pieceService, | ||
| final OutputView outputView | ||
| ); | ||
| } |
There was a problem hiding this comment.
-
ChessCommand 를 도입했을 때 장단점이 있을 것 같은데, 이 부분에 대해서 다즐의 생각이 궁금합니다.
-
커맨드 패턴을 Controller Level 에서 사용해주셨는데요! Domain Level 에서 커맨드 패턴을 사용하는 관점도 한 번 고민해보시면 좋을 것 같아요!
(view, service 를 호출하는 것이 아닌 Domain 의 상태만을 변경하도록)
There was a problem hiding this comment.
커맨드를 도입 후 장단점은 아래와 같습니다.
장점
- 새로운 커맨드가 추가되더라도 확장에 편리하였습니다.
- 각각의 커맨드가 온전히 본인의 역할만 수행할 수 있었습니다.
단점
- 다른 커맨드에서 사용되기에 필요하지 않은 매개변수를 받아야하는 상황이 발생하였습니다.
- 전체적으로 구조를 이해하는데 어려움이 존재합니다.
제가 체스 미션에서 적용한 이유는 학습을 위한 목적도 있었으며, 커맨드를 적용하더라도 각각의 역할이 잘 드러나있다고 판단하여 이해하는데 어려움이 없어 적용하도록 결정하였습니다!
두 번째 질문은 view와 service를 매개변수로 넘기는 것이 아니라 ChessGame처럼 ChessController도 상태를 가지고 상태를 변경하며 동작할 수 있도록 고민해보라는 질문일까요? 명확하게 이해하지 못했습니다 😂
There was a problem hiding this comment.
다즐이 생각하는 장단점 답변 감사합니다! 객체 지향을 넘어 계층에 관해 고민하는 부분이 인상 깊습니다! 👍
계층에 대해서도 사람마다 생각하는 바가 다르겠지만, 제 의견 남겨봅니다!
여러 계층이 도입된다고 하더라도, 제가 생각하는 코드에서 가장 중요한 점은 '도메인 로직'이 명확하게 보이냐 입니다. 레벨1 동안 계속 고민해왔던 부분이고요.
DB 로직이 추가된다면, 도메인 로직이 실행된 이후 db.save(chess) db 저장의 책임만을 분리할 수 있지 않을까 생각해요.
db 에 어떤 방식으로 저장할지에 대한 것은 저장 계층의 책임이지 Controller 가 db의 구성을 다 알고 개별의 dao, service 를 각각 호출 명령을 내리는 것은 너무 많은 책임을 Controller 에서 하고 있는 것이라 생각합니다.
There was a problem hiding this comment.
컨트롤러의 역할이 많아지는 느낌이 들었는데 완태가 설명해주신 내용이 답이 될 것 같습니다! 해당 방향으로 수정해보겠습니다 :)
| import java.util.List; | ||
| import java.util.stream.Collectors; | ||
|
|
||
| public class ChessGameService { |
There was a problem hiding this comment.
Service 와 Dao 는 어떤 차이가 있으며, 생각하시는 서비스 계층의 역할은 무엇인가요? 또한 그 장단점에 대해서도 듣고 싶습니다.
- 서비스 계층을 도입하여 도메인의 변화가 최소한으로 이루어지도록 코드를 작성하였습니다.
이 부분에 대해선 추가 설명 요청드립니다. 서비스의 추가가 도메인을 보호한다라는 의미일까요?
Controller 에서 Dao 를 직접 사용했을 때보다 어떤 장점이 있을까요?
There was a problem hiding this comment.
Dao는 DB와 직접적으로 접촉하는 계층으로 DB에서 커넥션을 얻고 쿼리를 날려 데이터를 가져오는 역할이라고 생각합니다. Service 계층은 하나의 논리적인 단위의 로직에 사용되는 여러 Dao를 조합하여 연산을 수행하는 계층이라고 생각하였습니다. 내용을 작성하고보니 ChessGame, Piece 각각 Service를 두지 않고 하나의 Service로 관리해도 괜찮을 것 같다는 생각이 들었습니다!
장점으로는 논리적인 연산을 추상화시켜 의미있는 결과를 만들어낼 수 있다고 생각합니다. 단점은 크게 떠오르지 않는 것 같은데 어떤 단점이 존재할까요 🤔
Dao를 도메인에 넣지 않고 Service 계층에서 사용하여 도메인에 Dao를 넣지 않고 하나의 로직처럼 사용하고자 했음을 표현하였습니다. Controller에서 도메인과 DB 로직을 하나로 묶기 보다는 서비스 계층에서 하나의 논리적인 단위로 묶어 처리하고자 했습니다! 컨트롤러는 서비스의 추상화된 로직만 알면된다는 장점이 존재한다고 생각합니다.
There was a problem hiding this comment.
하나의 논리적인 단위의 로직 이라는 부분이 지금의 구조에서는 dao 의 값을 전달하는 역할 위주로 하는 것 같아, '논리의 단위'가 칭하는 것이 무엇인지 궁금합니다.
- 체스 게임 생성
- 말 이동
- 체스 게임 종료
등이 논리적인 단위가 될 수 있지 않을까요?
There was a problem hiding this comment.
db 조회, 저장에 대한 생각을 추가로 남기면
'db 조회 => domain 변화 => db 저장' 이 흐름이 한 눈에 보였을 때 저는 이해하기 쉽더라고요!
이러한 흐름이 명확하게 보이는 방식으로 리팩터링 해보면 어떨지 의견남깁니다!
There was a problem hiding this comment.
리뷰 확인 후 지금 service 객체를 제대로 활용하지 못하고 있다는 것을 확인할 수 있었습니다 ..
db 조회 => domain 변화 => db 저장 이라는 흐름이 잘 보이도록 리팩토링해고자 합니다! 감사합니다
| private void checkKing(final ChessGame chessGame) { | ||
| if (chessGame.isKingDied()) { | ||
| chessGame.end(); | ||
| pieceService.deleteAll(chessGame.getId()); | ||
| chessGameService.delete(chessGame.getId()); | ||
| outputView.printDoneMessage(); |
There was a problem hiding this comment.
게임이 끝났을 때, db 에서 삭제하는 부분은 어색하다 생각합니다.
게임 별로 상태를 갖도록 하는 것은 어떨까요?
만약 상태를 추가한다면, 해당 로직은 도메인에서 진행되어야 한다 생각합니다.
There was a problem hiding this comment.
왕이 죽은 후에는 기물이 움직일 수 없기에 DB에서 삭제해도 괜찮지 않을까라는 판단을 하였습니다. 움직일 수 없더라도 왕이 죽은 상태라고 표시 후 사용자가 해당 게임에 접근했을 때 결과를 알려주는 방법도 고민해 보았는데, 이런 의도를 가지고 설명해주신건지 궁금합니다!
There was a problem hiding this comment.
실제 현업에서는 row를 삭제하는 경우가 매우매우 드물기도 하고,
실제 게임 서비스라면, 더욱더 게임이 종료된 경우에는 그 상태를 저장하고 있지 않을까 해서 남긴 리뷰였습니다!
가능하면, 실제 row를 삭제하기 보다는 상태를 업데이트하는 방식으로 진행해보는 것은 어떨까요?
There was a problem hiding this comment.
좋은 것 같습니다! 게임이 종료되더라도 유의미한 데이터가 될 수 있을 것 같아요 👍
src/docker/data/init.sql
Outdated
| CREATE TABLE chess_game | ||
| ( | ||
| `id` INT NOT NULL AUTO_INCREMENT, | ||
| `turn` VARCHAR(20) NOT NULL, | ||
| PRIMARY KEY (`id`) | ||
| ); | ||
|
|
||
| CREATE TABLE piece | ||
| ( | ||
| `id` INT NOT NULL AUTO_INCREMENT, | ||
| `chess_game_id` INT NOT NULL, | ||
| `color` VARCHAR(20) NOT NULL, | ||
| `type` VARCHAR(20) NOT NULL, | ||
| `file` VARCHAR(20) NOT NULL, | ||
| `rank` VARCHAR(20) NOT NULL, | ||
| PRIMARY KEY (`id`), | ||
| FOREIGN KEY (`chess_game_id`) REFERENCES `chess_game` (`id`) | ||
| ); |
There was a problem hiding this comment.
chess_game 과 이를 이루는 piece들을 1:N 관계를 유지했네요 👍
혹시 고민해본 다른 저장방식이 있을까요? DB 설계가 매우 중요하기 때문에, 어떤 논리, 생각으로 지금의 구조를 결정하게 되었는지 궁금합니다. 👀
There was a problem hiding this comment.
다른 크루들과 얘기를 해보았을 때, 움직인 기보를 저장하는 방식도 고려해 보았습니다! 해당 방법은 움직임을 저장하고 과거 기보를 확인할 수 있다는 장점이 있지만 폰이 몇 개 존재하는지, 블랙 기물을 몇 개 존재하는지 등의 DB에서 추출할 수 있는 유의미한 데이터를 가지지 못한다는 판단을 하였습니다. 따라서 위와 같은 구조로 테이블을 설계 후 미션을 진행해 보았습니다!
There was a problem hiding this comment.
Square와 Piece도 분리해볼까 고민해보았으나 1:1 구조에서 어느 테이블에 FK를 두어야할지 명확한 기준이 서지 않아 하나의 테이블로 진행하였습니다. 분리하게 된다면 어느 곳에 FK를 두는 것이 좋을지 궁금합니다!
추가적으로 테이블을 나누게 되면 추가적인 join 연산이 발생하게 되는데 테이블을 명확하게 분리하기 위해 조인이 추가되더라도 분리하는게 좋을지도 궁금합니다!
There was a problem hiding this comment.
생각 공유 감사합니다! 👍
지금의 방식의 장점은 db select 를 통해서 현재 게임의 진행상황을 쉽게 확인할 수 있는 장점이 있겠네요.
말씀해주신 '움직인 기보를 저장하는 방식' 은 '이벤트 소싱' 방식으로도 불리우는데, 실제 게임에서 많이 사용하는 방식입니다. (리그오브레전드를 해보셨을 지 모르겠지만, 게임을 나갔다 들어오면 로딩되는데, 이때 모든 이벤트 들이 입력되는 것으로 알고 있습니다.) 이 방법은 모든 히스토리를 다 남기고 있어서, '한 번 무르기', 등이 가능해보이네요
이외에도 현재 board 의 상태를 json 화 해서 그냥 db 에 짱박는 방식도 있습니다. 이 방법의 장점은 아마 테이블 하나로 관리할 수 있다? 정도겠네요!
There was a problem hiding this comment.
이벤트 소싱이라는 방법도 존재하는군요! 만약 해당 방식을 사용하게 된다면 마지막 이벤트를 진행한 뒤의 상태로 만들기 위해 DB에 있는 모든 이벤트를 불러와서 클라이언트에서 이벤트를 실행시키는 작업이 필요할 것으로 생각되는데, 클라이언트에 부담이 가게 되지 않나요? 🤔
마지막 이벤트 후의 상태를 캡처해서 따로 저장해두는 식으로 상태를 유지시키는걸까요?
There was a problem hiding this comment.
이벤트 소싱 또한 그 구현마다 다양하게 적용할 수 있을 것 같아요.
- client 가 모든 이벤트 소싱을을 개별적으로 실행
- server에서 이벤트 소싱을 실행한 결과를 내려줌.
- 조금 더 최적화된 관점은 이벤트 소싱 중간의 snapshot 을 저장하고 해당 snapshot 시점부터 이벤트 소싱 실행
관련해서 재밌게 본 영상 공유드려요. https://www.youtube.com/watch?v=FUEZibcZEkg part2 확인해보세요!
| @@ -44,17 +42,24 @@ private ChessGame readChessGame() { | |||
| private void playChessGame(final ChessGame chessGame) { | |||
| try { | |||
| final ExecuteCommand executeCommand = inputView.readExecuteCommand(); | |||
| executeCommand.execute(chessGame, chessGameService, pieceService, outputView); | |||
| executeCommand.execute(chessGame, chessGameService, outputView); | |||
| } catch (IllegalArgumentException | IllegalStateException e) { | |||
| outputView.printErrorMessage(e.getMessage()); | |||
| } | |||
| } | |||
|
|
|||
| private void checkKing(final ChessGame chessGame) { | |||
There was a problem hiding this comment.
checkKing 의 로직이 서비스에 들어갈 수 있지 않을까? 생각되네요! (move 메소드 뒤에?)
도메인 객체를 주는 범위를 Service Layer 를 도입한다면, Service 에서만 진행한다면 좋지 않을까 생각해요! (상태 변화를 확인하기 위해서 Service 코드만 확인할 수 있을 것 같습니다!
| public void saveChessGame(final ChessGame chessGame) { | ||
| chessGameDao.save(chessGame); | ||
| final ChessGameDto chessGameDto = chessGameDao.findLatest() | ||
| .orElseThrow(() -> new IllegalStateException("저장된 게임이 존재해야 합니다.")); | ||
| chessGame.setId(chessGameDto.getId()); | ||
| savePieces(chessGame); |
There was a problem hiding this comment.
| public void saveChessGame(final ChessGame chessGame) { | |
| chessGameDao.save(chessGame); | |
| final ChessGameDto chessGameDto = chessGameDao.findLatest() | |
| .orElseThrow(() -> new IllegalStateException("저장된 게임이 존재해야 합니다.")); | |
| chessGame.setId(chessGameDto.getId()); | |
| savePieces(chessGame); | |
| public void saveChessGame(final ChessGame chessGame) { | |
| chessGameId = chessGameDao.save(chessGame); | |
| final Map<Square, Piece> table = chessGame.getBoard(); | |
| for (final Square square : table.keySet()) { | |
| pieceDao.save(chessGameId, square, table.get(square)); | |
| } |
id 값을 구하기 위해 복잡해진 것 같네요! 아래 방식을 통해 dao level에서 id 를 반환하거나, id 값이 추가된 객체를 반환해볼 수도 있을 것 같아요! (db 로직을 위해 서비스가 복잡해진 것으로 보입니다)
https://www.springcloud.io/post/2022-06/jdbctemplate-id/#gsc.tab=0
| import chess.domain.game.state.StartState; | ||
| import java.util.Map; | ||
|
|
||
| public class ExecuteStateSerialization { |
There was a problem hiding this comment.
Enum 값은 별도의 serialization 을 구현하기 보다는 ExecuteState.name() 으로도 관리할 수 있을 것 같네요!
안녕하세요 완태 🏀
DB를 많이 다루어보지 못해서, 생각보다 오래걸리게 되었네요. 잘 지내셨나요?데이터베이스는
docker디렉토리에 존재하는docker-compose파일을 실행하면 자동으로init.sql이 실행되도록 설정해두었습니다. 실제 동작을 확인하고 싶으시면 실행 후 확인해주시면 감사합니다!이번 미션에서 진행한 내용을 정리해 보았습니다.
1. 입력 커맨드에 따른 동작을 수행하도록 하였습니다.
2. 체스 게임은 상태에 따라 알맞은 동작을 수행하도록 하였습니다.
3. 기물이 Piece를 상속하는 구조를 제거하고 PieceType을 가지도록 수정하였습니다.
4. 체스 게임 테이블과 체스 기물 테이블을 설계하고 DB에 게임 상태가 저장되도록 하였습니다.
5. 여러 게임을 저장할 수 있게 체스 게임에 ID를 부여하였습니다.
6. 사용자는 저장된 여러 게임 중 게임 ID를 입력하여 원하는 게임을 플레이할 수 있습니다.
7. 체스 기물이 움직일 때마다 체스 게임과 기물 정보가 업데이트될 수 있도록 하였습니다.
8. 왕이 잡혀 게임이 종료된 체스 게임은 DB에서 데이터가 삭제될 수 있도록 하였습니다.
9. 템플릿 콜백 패턴을 사용하여 코드 중복을 줄이도록 하였습니다.
10. 서비스 계층을 도입하여 도메인의 변화가 최소한으로 이루어지도록 코드를 작성하였습니다.
그 외에도 몇 가지 작은 수정사항을 반영하고 리팩토링을 진행해보았습니다!
한 가지 궁금한 점도 존재하였습니다 🤔
PieceTable도Piece와Square테이블로 나누어보려고 하였으나, 둘의 관계는1:1이기에 어느 테이블에서FK를 가져야할지 고민이 되어, 결국 하나의 테이블로 설계를 진행하고 코드를 작성하였습니다. 테이블의 관계가1:1인 경우 어떻게 합리적으로FK를 가질지 선택할 수 있는 방법이 궁금합니다. 위와 같은 상황일 때 어떤 선택을 하실지도 궁금합니다!벌써 Level1 마지막 PR이라니 시간이 참 빠른 것 같습니다. 얼마전 우연히 우테코 인스타로 인터뷰하신 내용을 봤는데 제 리뷰어분이었다는 것에 일단 놀랐고, 좋은 내용이 많이 담겨있어서 많은 생각과 고민을 할 수 있었던 것 같습니다. 좋은 영향력을 펼치고 있는 모습 저도 본받고 싶습니다. 이번 리뷰도 잘 부탁드리겠습니다. 감사합니다 :)