[3, 4단계 - 체스] 성하(김성훈) 미션 제출합니다. #556
Conversation
이동 기능 매개변수로 Rank,File 별 이동 횟수를 전달받도록 수정
이동 기능 매개변수로 Rank,File 별 이동 횟수를 전달받도록 수정
| private JdbcChessGameDao() { | ||
| } | ||
|
|
||
| public static JdbcChessGameDao getInstance() { |
There was a problem hiding this comment.
✅ DAO 싱글톤 질문
DAO에 관해서 구글링 하다가, 싱글톤으로 많이 사용한다는 것을 듣고 적용해봤습니다.
DAO가 사용될 때마다 새로운 인스턴스를 생성하는 것은 낭비이기 때문에 이러한 점에서는 싱글톤이 효과적이라고 생각했습니다!
그런데 현재 DAO를 사용하는 곳은 각 상태들에서 사용하고 있습니다.
이때 싱글톤을 적용하여 static 전역으로 생성하게 되면
다른 객체들도 DAO를 사용할 수 있게 되어 캡슐화 측면에서 좋지 않아보였습니다.
이러한 캡슐화가 깨지는 측면은 상관이 없는 걸까요?
There was a problem hiding this comment.
캡슐화의 정확한 의미가 무엇일까요? 또한 static 키워드는 왜 캡슐화를 깨진다고 생각하셨는지 성하의 의견을 코멘트로 적어주세요!
그리고 캡슐화와는 별개로 static를 사용하거나 사용하지 않거나와 관계 없이 다른 객체가 DAO는 사용할 수 있지 않나요?
There was a problem hiding this comment.
제가 생각하는 캡슐화는 '객체를 사용하기 전까지는 객체 내부의 정보들을 모르게 하는 것'이라고 생각했어요!
저는 '객체를 사용한다는 것'이 객체의 인스턴스를 만들어서 사용하는 것이라고 이해해서
static은 객체를 사용하지 않고 객체 내부의 정보를 알게 해서 캡슐화가 깨진다고 이해한 것 같아요!
그래서 static을 붙이지 않으면 객체의 인스턴스를 생성해야 DAO를 사용할 수 있기 떄문에
'난 DAO를 사용할 거야!' 라는 행동을 객체의 인스턴스 생성으로 봤습니다.
static으로 선언하면 'DAO를 사용할거야!'라는 행동 없이도 직접 객체 안의 메소드들을 볼 수 있기 때문에 위험하다고 생각했어요!
There was a problem hiding this comment.
캡슐화는 객체의 상태와 행위를 묶고 실제 구현 내용 일부를 외부에 감추어 은닉하는 객체 지향 패러다임의 원칙 중 하나입니다!
위 정의에 따르면 static을 사용하여 객체의 행위에 접근한다고 하더라도 구현 내용을 공개하는 것은 아니기 때문에 static이 캡슐화를 깨뜨리지는 않다고 생각해요 ㅎㅎ 다만 static은 캡슐화와는 별개로 객체의 상태를 사용할 수 없고 다형성과 같은 객체 지향의 장점을 온전히 누릴 수 없고, 메모리에 시작부터 올라가서 해제하기 어렵다는 점에서의 단점이 있습니다.
There was a problem hiding this comment.
안녕하세요 성하!
이번 미션에 DAO와 상태 패턴이라는 난이도 있는 작업을 잘 작업해 주셨네요 👍
코멘트를 작성하다 보니 구조적으로 큰 변화가 있을 것 같아서 일단 이정도만 피드백을 반영해보시면 좋을 것 같습니다 ㅎㅎ
추가로 PR 본문에 남겨주신 질문은 여기에 답변 드릴게요!
Q. 움직이는 방식(ex. MoveStrategy)을 인터페이스로 하는 방식으로 리팩토링 하지 않은 이유
A. 성하가 그렇게 생각하셨다면 그대로 진행하셔도 됩니다 😊
Q. 상태 패턴 적용
A. 말씀 주신대로 상태 별로 고유한 동작만 나눠서 가질 수 있어서 필요한 코드만 둘 수 있다는 장점이 있습니다. 그리고 단점으로는 상태가 늘어날 때마다 기존 상태 코드들의 코드도 변경해야한다고 적어주셨는데, 오히려 상태 패턴으로 상태를 구분해 두었으니 새로운 상태가 추가되면 기존 코드는 거의 영향이 없어야 하는 것 아닐까요? ㅎㅎ
| private JdbcChessGameDao() { | ||
| } | ||
|
|
||
| public static JdbcChessGameDao getInstance() { |
There was a problem hiding this comment.
캡슐화의 정확한 의미가 무엇일까요? 또한 static 키워드는 왜 캡슐화를 깨진다고 생각하셨는지 성하의 의견을 코멘트로 적어주세요!
그리고 캡슐화와는 별개로 static를 사용하거나 사용하지 않거나와 관계 없이 다른 객체가 DAO는 사용할 수 있지 않나요?
|
안녕하세요 제이온!! 제이온 덕분에 DAO와 Entity에 대해 더 알게 된 것 같아요! 😃 🎯 제이온의 피드백을 반영한 주요 리팩토링제이온의 피드백을 반영한 주요 변경점은 다음과 같습니다! 1. DAO에 DTO 대신 Entity를 전달하기 (Entity에서 빌더 패턴 사용)
2. DAO와 도메인을 가지는 ChessGameService 서비스 계층 생성
3. ChessGameDao -> 게임 관련 ChessGameDao / 기물 관련 PieceDao로 분리DAO와 서비스 계층, 상태 패턴에 익숙하지 않다보니까 이렇게만 했는데도 좀 오래 걸렸네요 ㅠㅠ 😭 🎯 리팩토링 진행 시 궁금했던 점다음은 리팩토링을 진행하면서 궁금했던 점을 질문해보겠습니다!! ✅ 1. Domain인 ChessGame을 상태가 가질지 Service가 가질지 고민해당 고민을 하느라 엄청 시간을 많이 썼던 것 같습니다.
리팩토링 전에는 제이온이 말해준 것처럼 상태 객체들이 많은 역할을 하게 되었습니다. ChessGame을 상태가 가진다면, 결국 상태들이 도메인과 서비스를 같이 가져서 위와 같은 역할을 다시 모두 담당하게 되는 것 같았습니다. 그래서 서비스 계층에 Domain인 ChessGame과 DAO를 두게 되었습니다. 서비스 계층이 DAO만 가지고 있는 것이 아니라 Domain을 가져도 괜찮은지 궁금합니다!! 나머지 질문은 코드 관련 질문이라서 코드에 코멘트로 남기겠습니다!! 🙇🏻♂️ 끝까지 리뷰해주셔서 정말 감사합니다 제이온!! ㅎㅎㅎ 😃 |
| private JdbcChessGameDao() { | ||
| } | ||
|
|
||
| public static JdbcChessGameDao getInstance() { |
There was a problem hiding this comment.
캡슐화는 객체의 상태와 행위를 묶고 실제 구현 내용 일부를 외부에 감추어 은닉하는 객체 지향 패러다임의 원칙 중 하나입니다!
위 정의에 따르면 static을 사용하여 객체의 행위에 접근한다고 하더라도 구현 내용을 공개하는 것은 아니기 때문에 static이 캡슐화를 깨뜨리지는 않다고 생각해요 ㅎㅎ 다만 static은 캡슐화와는 별개로 객체의 상태를 사용할 수 없고 다형성과 같은 객체 지향의 장점을 온전히 누릴 수 없고, 메모리에 시작부터 올라가서 해제하기 어렵다는 점에서의 단점이 있습니다.
| public interface CommandStatus { | ||
|
|
||
| CommandStatus start(); | ||
|
|
||
| CommandStatus restart(Long previousGameId); | ||
|
|
||
| CommandStatus move(Position sourcePosition, Position targetPosition); | ||
|
|
||
| CommandStatus end(); | ||
|
|
||
| CommandStatus printGameResult(); | ||
|
|
||
| boolean isEnd(); | ||
|
|
||
| boolean canPrintGameResult(); | ||
|
|
||
| List<Piece> getPieces(); | ||
|
|
||
| String getTurnDisplayName(); | ||
|
|
||
| ScoreBySideDto getScoreBySide(); | ||
|
|
||
| GameResultBySideDto getGameResultBySide(); | ||
| } |
There was a problem hiding this comment.
지금 Init이나 End 등 CommandStatus 하위 객체들은 위 메소드를 실질적으로 모두 필요로 하지 않습니다.
SOLID 원칙 중 ISP 원칙에 대해 학습해 보시고 적용해보면 어떨까 합니다!
참고할 만한 아티클을 공유드릴게요.
There was a problem hiding this comment.
이 피드백은 선택적이어서 반영안하셔도 무방하지만, 추후 미션을 하시면서 범용 인터페이스보다는 필요에 맞게 분산이 가능할지 고민해보시면 많은 도움이 됩니다! (물론 이건 저도 늘 어렵습니다 ㅋㅋㅋ)
|
제이온 안녕하세요! 리뷰이 성하입니다!! 🙇🏻♂️ 이번에 정말 DM으로 많이 질문을 한 것 같아서 죄송합니다 ㅠㅠ 🥹 그래도 빠르게 답변해주시고 친절하게 해주셔서 정말 감사했습니다... 🥹 🎯 주요 리팩토링이번 주요 리팩토링은 다음과 같습니다! ✅ 1. Entity 일부 삭제 & 일부 적용단순히 도메인 getter로 사용할 수 있는 로직들에서는 Entity를 삭제하고, gameId로 모든 Piece를 가져오는 또, gameId로 ChessGame을 가져오는 따라서 chess_game 테이블의 id, turn만 받아와서 Entity를 DAO 단에서 만들고 제이온이 없었으면 Entity를 생각 못 했을 것 같은데 정말 감사합니다 ㅎㅎㅎ ✅ 2. Init 시 ChessGameService 생성 리팩토링Init 시에 DAO의 |
| public class ChessGameEntity { | ||
|
|
||
| private long id; | ||
| private String turn; |
There was a problem hiding this comment.
앞으로 미션 수행하실 때 이렇게 테이블과 매핑되는 Entity 객체를 만들어두면 다른 개발자가 DB를 직접 까서 테이블을 확인하지않더라도, 테이블 스키마를 확인할 수 있다는 장점도 있습니다 ㅎㅎ
| private final ChessGameDao chessGameDao; | ||
| private final PieceDao pieceDao; | ||
|
|
||
| public static ChessGameService createInitChessGameService() { |
| public static ChessGameService createInitChessGameService() { | ||
| ChessGameDao chessGameDao = JdbcChessGameDao.getInstance(); | ||
| long recentGameId = chessGameDao.findRecentGameId(); | ||
| ChessGame chessGame = new ChessGame(++recentGameId, new Board(new Pieces()), Turn.WHITE); |
There was a problem hiding this comment.
�레벨 2를 하면서 ID를 조금 더 신경써서 설계해주시면 좋을 것 같습니다! ID는 식별자이니 유일해야 합니다.
| public interface CommandStatus { | ||
|
|
||
| CommandStatus start(); | ||
|
|
||
| CommandStatus restart(Long previousGameId); | ||
|
|
||
| CommandStatus move(Position sourcePosition, Position targetPosition); | ||
|
|
||
| CommandStatus end(); | ||
|
|
||
| CommandStatus printGameResult(); | ||
|
|
||
| boolean isEnd(); | ||
|
|
||
| boolean canPrintGameResult(); | ||
|
|
||
| List<Piece> getPieces(); | ||
|
|
||
| String getTurnDisplayName(); | ||
|
|
||
| ScoreBySideDto getScoreBySide(); | ||
|
|
||
| GameResultBySideDto getGameResultBySide(); | ||
| } |
There was a problem hiding this comment.
이 피드백은 선택적이어서 반영안하셔도 무방하지만, 추후 미션을 하시면서 범용 인터페이스보다는 필요에 맞게 분산이 가능할지 고민해보시면 많은 도움이 됩니다! (물론 이건 저도 늘 어렵습니다 ㅋㅋㅋ)
안녕하세요 제이온!! 성하입니다! 😃
저는 크루 중에 Merge가 빨리 된 편인데 3단계 들어가기 전에 여러 가지 리팩토링을 거치는데 시간을 많이 썼습니다!
또, 3단계 4단계도 구현이 어려워서 시간이 엄청 오래 소요된 것 같습니다!
3단계, 4단계에 들어가기 전 리팩토링한 것과 3단계, 4단계에서 궁금한 점들을 질문드리겠습니다!! 🙇🏻♂️
1, 2단계 리팩토링
1. 움직이는 방식(ex. MoveStrategy)을 인터페이스로 하는 방식으로 리팩토링 하지 않은 이유
제이온이 해당 방식을 추천해주셨는데, 제 생각이 부족했을 수도 있지만 제가 설계했을 때
이렇게 진행할 경우 RookMoveStrategy 같은 구현체들이 Position을 가져야했습니다!
저는 전략들이 위치를 아는 것은 조금 어색하다고 생각하여 원래 구조를 유지하게 됐습니다!
그래도 생각해볼만한 접근이었던 것 같아요! 감사합니다!! 👍
2. 상태패턴 적용
이전 블랙잭 미션 때 상태 패턴을 처음 들어봤지만, 시간이 없어서 미션에 적용해보지는 못했습니다.
그래서 이번에 Merge가 빨리 되기도 하고, 3단계에서
status커맨드가 추가되는 것을 보고반복되는 if문을 줄일 수 있는 상태 패턴을 학습 목적으로 적용해봤습니다!
CommandStatus를 관리하는CommandManager가 존재하고,CommandStatus는 다음과 같이 4가지 상태가 존재합니다.제가 익숙하지 않아서 그런지 상태 패턴 적용하는 데에만 2~3일을 썼던 것 같아요 ㅠㅠ 😭
제가 상태 패턴을 적용하면서 느낀 장점은 각각 상태별로 동작하는 로직이 다르기 때문에
동작하는 로직에 필요한 것만 필드로 둘 수 있다는 점이었습니다.
컨트롤러에 ChessGame을 두게 되면, 처음 애플리케이션이 실행되는 시점에 보드가 생성되어야 했습니다.
하지만 상태 패턴을 적용해보니 Init 상태에서는
ChessGame을 두지 않아서 애플리케이션 시작 시점에 보드가 생성되지 않게 되었습니다.Play, PrintGameResult 상태에서만
ChessGame이 필요하여 해당 상태에서만ChessGame을 들고 있다는 점이 좋았던 것 같아요!또, 상태별로 관리하다보니 이후의 3, 4단계 구현 시에 상태별로 편하게 DAO를 적용했던 것 같습니다!
제가 느낀 상태 패턴의 단점으로는 상태 하나의 동작이 추가될 때마다
해당 상태의 동작만 추가하면 되는 것이 아니라 모든 상태의 동작을 추가하게 되어 유지 보수가 힘들 것 같았습니다!
제가 상태 패턴을 제대로 적용한건지, 제가 생각한 상태 패턴의 장단점이 적절한지 궁금합니다!!
3, 4단계 구현
1. 3단계
3단계는
ResultCalculator라는 결과 계산 객체를 둬서진영별 점수와진영별 게임 결과를 관리하도록 했습니다!3단계는 구현이 좀 걸렸지만 질문할 것은 없는 것 같아요!!
2. 4단계
4단계에서는 DB 설계와 DAO 사용이 제일 힘들었던 것 같아요.
제가 설계한 테이블 구조는 다음과 같습니다!
piece테이블에서chess_game의 game_id를 FK로 가지게 설계하였습니다.게임이 정상적으로 종료되지 않고 재시작 할 때,
restart라는 커맨드를 추가하여 재시작 할 게임의 ID를 받아서 재시작하게 하였습니다.동작은 다음과 같습니다!
DB 설계와 DAO 사용에 관한 질문들이 많은데 코드 단에 코멘트로 남겨두도록 하겠습니다!!
항상 좋은 리뷰 감사합니다 제이온!! 이번 3, 4단계도 잘 부탁드립니다! 🙇🏻♂️