[3, 4단계 - 체스] 이레(이다형) 미션 제출합니다. #606
Conversation
There was a problem hiding this comment.
안녕하세요 이레.
전체적으로 Dao와 Service 구조는 잘 잡아주신것 같아요. 다만 도메인 로직이 아직은 완성되지 않은것 같아서 관련해서 피드백 남겨놓았습니다. 지난번 리뷰때 남겼던 피드백들을 모두 반영해주셨는지 한번 확인해주세요.
그리고 테이블 설계에 대해서 한번 더 고민해보시면 좋을것 같아요. 현재는 매번 최신 gameId만 불러오기 때문에 게임이 여러개가 되었을때 기존 게임을 불러올 수 없고, 매번 불러올때마다 move를 일일히 전부 해줘야한다는 단점이 있습니다. 물론 이부분은 요구사항은 아니기 때문에, 고민만 해보셔도 좋을것 같아요.
그리고 DB에 게임정보가 없을때 새 게임을 시작해야하는데, 지금은 예외가 발생하는것 같아서 확인한 번 해주시면 좋을것같네요.
| @@ -0,0 +1,18 @@ | |||
| version: "3.9" | |||
There was a problem hiding this comment.
지금 도커 관련된 파일이 git에 등록된것 같은데, 제외해 주셔야합니다.
| this.connectionDriver = new ConnectionDriver(); | ||
| } | ||
|
|
||
| public Optional<Integer> findLastGameIdByStatus(String status) { |
| this.chessBoard.initialize(boardStrategy.generate()); | ||
| stateOfChessGame = StateOfChessGame.MOVING; |
| this.movementDao = movementDao; | ||
| } | ||
| private static final int NONE_GAME_ID = -1; | ||
| private int gameId = NONE_GAME_ID; |
There was a problem hiding this comment.
ChessGame의 아이디는 ChessGame 도메인이 갖는게 맞을것 같아요.
service가 이걸 가져야하는 이유가 있나요?
| } | ||
|
|
||
| /** | ||
| * todo : (String start, String end)로 변경하기! | ||
| * 질문: Position생성을 chessGame에서 하는 게 맞나요 아니면, |
There was a problem hiding this comment.
GameService에서 요청을 도메인객체로 변환해주는 작업을 책임지는게 어떨까요?
| import java.sql.Statement; | ||
| import java.util.Optional; | ||
|
|
||
| public class ChessGameDao { |
|
|
||
| import java.util.Arrays; | ||
|
|
||
| public enum CrossDirection { |
| checkMovableDirection(direction); | ||
| checkMovableDistance(start, end); | ||
| checkMovableDestination(destinationColor, direction);// | ||
| public double getScore(final List<Piece> piecesInSameColumn) { |
There was a problem hiding this comment.
Board의 세로 칼럼에 다른 피스를 전달받는거 자체가, piece가 너무 많은 Board 구현을 알고 있다는 생각이 드네요.
Board와 Piece 객체간의 관계를 좀 더 잘 설계해야할것 같습니다.
| Piece pieceToMove = findPieceInBoardByPosition(start); | ||
| checkIfPieceToMoveEmpty(pieceToMove); | ||
| checkIfPieceMovable(start, end, pieceToMove); | ||
| checkIfOtherPiecesOnPath(start, end); |
|
|
||
| import static chess.domain.game.StateOfChessGame.*; | ||
|
|
||
| public class ChessGameService { |
|
안녕하세요 디디 레벨인터뷰와 겹치게 되면서 리팩터링이 많이 늦어졌습니다. 리뷰 주신 것과 리팩터링을 하며 생긴 궁금점이 몇가지 있는데, 함께 답변해주시면 정말 감사하겠습니다! 리뷰1)
리뷰2) #606 (comment)
리뷰3) #606 (comment) 리뷰4) Knight� 건너뛸수 있는게 가능한 상태인가요? |
There was a problem hiding this comment.
안녕하세요 디디
레벨인터뷰와 겹치게 되면서 리팩터링이 많이 늦어졌습니다.
특히, 게임 방을 구현하고 싶어서 table도 만들고 request 등 다른 구조도 많이 만들었는데, 최종적으로 gameId(== roomId)를 어디에 저장해야할지 결정하지 못해 모두 지우고 필수 요구사항만 지킨 데이터 베이스를 완성하게 되었습니다.
리뷰 주신 것과 리팩터링을 하며 생긴 궁금점이 몇가지 있는데, 함께 답변해주시면 정말 감사하겠습니다!
안녕하세요 이레.
말씀해주신것처럼 gaemId는 ChessGame이 가지면 된다는 생각입니다.
사용자로부터 불러오고 싶은 gaemId를 입력받게 하고요.
리뷰1)
저는 Piece를 단위로 저장하면 될거라고 생각했어요.
gameId, pieceType, pieceColor, Position 이런식으로요.
리뷰2)
체스서비스가 체스게임을 필드로 가질필요가 있나요?
dao에서 gameId로 board와 status를 불러와서 chessGame을 생성해주면 되는거 아닐까요?
리뷰3)
저는 board에서 isSamePieceOnColumn() 인지를 판단해서, piece 계산전략을 다르게 가져가면 어떨까 생각했어요.
더 나아가 이런 로직들을 전략패턴으로 숨길수도 있겠죠?
리뷰4)
이것도 마찬가지로, piece.canLeap() 정도만 구현해놓고, 장애물을 검사하는 board에서 예외를 던질지 말지를 결정하면 좋을것 같다고 생각했습니다.
객체의 설계에는 정답이 없기때문에 본인만의 기준을 만드시는게 중요합니다. 다만 객체가 자신이 할수있는 일만 하도록 설계하면 응집도 높은 코드를 작성하실 수 있을거에요.
요 피드백은 나중에 시간여유 되실때 한번 다시 보시면서 본인만의 기준을 세우시면 좋을것 같습니다.
어려운 미션 수행하시느라 고생많으셨습니다 :)
안녕하세요 디디!
이번 미션에서 sql과 도커를 처음 써봐서 코드 구조에 대해 깊이 신경쓰지 못했습니다.
리팩토링 할 부분이 많은, 부족한 코드가 되어버렸네요ㅠㅠ 테이블 구조나 dao service 설계 등 많이 지적 부탁드립니다:ㅇ
추가하여 몇 가지 궁금증이 있어 함께 첨부합니다. 답변해 주시면 감사하겠습니다!
ChessGame 클래스에서
move()메서드 실행 시, Command에 저장된 상수를 이용하여, Position객체를 생성합니다.
Position생성을 chessGame에서 하는 게 맞을까요? 아니면, Gameservice에서 뷰에서 넘어온 command(명령어)에 따른 Postion 객체 생성까지 책임지는 게 맞을까요?
고민하는 이유는, chessGame에서 command 클래스가 가지고 있는 상수를 가져와 쓰는게 이상하게 느껴집니다.
ChessService에서 gameId를 저장해도 될까요?
후에 추가할 restart를 실행시키기 전까지 gameId가 바뀌지 않을 것이라고 생각했기 때문입니다.
또, gameId는 db에 들어갈 쿼리의 한 종류라고 생각되어서 따로 필드로 두고 저장하였는데, 올바른 위치가 아니라면 어느 곳이 적절할까요?
3.빨리 코드를 작성하다보니 dto를 전혀 생성하지 못하였는데(뷰에서 command관련 dto를 생성하여) 앞으로 리팩터링을 하며 dao를 통해 데이터를 꺼낼 때 사용하려�합니다. 적절할까요?