-
Notifications
You must be signed in to change notification settings - Fork 415
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
[4, 5단계 - 체스] 케빈(박진홍) 미션 제출합니다. #215
Conversation
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.
케빈 안녕하세요!
마지막 단계도 구현 잘 해주셨네요 👍
질문 주신부분에 대해 답변 남겼어요. 확인 부탁드려요!
궁금한점 있다면 언제든 DM주세요!
this.roomRepository = roomRepository; | ||
} | ||
|
||
public List<RoomDTO> findAllRooms() throws SQLException { |
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.
DTO가 Service내부에서 쓰이는 것에 대해 어떻게 생각하시나요? 🤔
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.
음... 이걸 사실 고민을 많이했어요 데이브.
- 서비스 레이어에서 parameter로 dto를 받아도 될까?
- 서비스 레이어는 dto를 반환해도 될까?
여러 글을 참조했는데요. 파라미터로 받는건 몰라도 반환하는 것에 대해서는 다시금 생각해볼 여지가 있을 것 같아요.
ChessBoard chessBoard = generateDefaultChessBoard(); | ||
Histories histories = new Histories(chessRepository.findAllHistoriesByRoomId(roomId)); | ||
histories.restoreChessBoardAsLatest(chessBoard); |
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.
매번 초기화 하는 부분에 대해 질문 주셨는데요!
ChessBoard의 현재 캐싱하고 있다가, ChessBoard에 변화가 있을때만 갱신해주는 방법은 어떠신가요?
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.
쉽지 않겠지만 방법을 강구해봐야겠습니다. ㅎㅎ
response.type(RESPONSE_JSON); | ||
int roomId = Integer.parseInt(request.params(":roomId")); |
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.
중복되는 부분은 메소드로 분리 해도 좋을 것 같아요!
안녕하세요, 데이브 :) 중복되는 메서드 분리ChessController에서 중복되는 부분들을 메서드로 분리했어요.
private int parseRoomIdFromUrl(Request request, Response response) {
response.type(RESPONSE_JSON);
return Integer.parseInt(request.params(":roomId"));
}
private BoardDTO findLatestBoardByRoomId(int roomId) throws SQLException {
ChessBoard chessBoard = chessService.findLatestBoardByRoomId(roomId);
TeamType teamType = chessService.findCurrentTeamTypeByRoomId(roomId);
return BoardDTO.of(chessBoard, teamType);
} DTO가 서비스 레이어 내부에서 쓰이는 것에 관하여
라는 질문에 대해 https://www.slipp.net/questions/93 여러 아티클들을 참고했습니다. 그리고 데이브가 준 질문에 대해 '정답은 없다'고 생각했어요. (실무를 모르는 입장에서 제가 접근할 수 있는 자료들을 통해 내린 짧은 생각입니다! ㅎㅎ 😔) Service Layer가 Domain 모델을 반환할지 DTO를 반환할지는 상황에 맞게 취사선택하라는게 여러 글들의 요지였는데요.
제 기존 코드들은 공통적으로 Service Layer에서 DTO를 받고 DTO를 반환했습니다. DTO를 Domain으로, Domain을 DTO로 변환하는 위치를 꼭 컨트롤러가 아닌 서비스 레이어에서 해도 되지 않을까? 하고 조심스럽게 생각했어요. DTO는 레이어간 데이터 전달의 목적이기때문에 어디서 해도 크게 문제가 되지 않을 것 같고, 위 글들에 따르면 서비스에서 도메인 모델을 보호하는 겸 겸사겸사...? 😂 이번에 리뷰 반영할 때는 서비스 레이어들이 DTO가 아닌 도메인 모델을 반환하도록 코드를 수정했습니다. 두 방식을 모두 사용해보았지만 현재 제 수준에서 쉽게 결론을 지을 주제가 아니라고 느꼈습니다. 다만 DTO 반환이 아닌 도메인 반환을 통해, 비대했던 Service Layer의 메서드들이 간결해지고 한 번에 한 가지 일만 수행하도록 더욱 분리할 수 있었습니다. 😂😂 데이브는 어떤 생각을 하시는지 알려주신다면 제가 공부하는데 큰 도움이 될것같아요! ChessBoard 캐싱에 관해현재 1, 2, 3, .... k개의 방이 있으면 각 방마다 개별적인 체스 게임 플레이가 가능하도록 되어 있는데요.
대신 기존 ChessService의 코드에 중복이 심하다고 판단해서 중복되는 부분을 별도의 메서드로 분리했습니다. 또한 일전의 Service Layer는 DTO를 반환하다 보니 하나의 메서드가 여러 동작을 수행했었는데요. 예를 들어, 기존의
등 두 가지 일을 했습니다. 메서드는 한 가지 일만 잘 해야한다는 생각이 들었고, CQS라는 개념을 접하게 되었는데요. Command Query Separation
기존 ChessService 메서드들은 CQS를 어기고 있었고, 히스토리 이력만 업데이트하는 명령 메서드로 리팩토링했습니다.
이후 컨트롤러에서 조회 쿼리 메서드를 통해 View에서 사용할 Board 객체를 찾아 응답하도록 했습니다. public JsonElement move(Request request, Response response) throws SQLException {
int roomId = parseRoomIdFromUrl(request, response);
MoveRequestDTO moveRequestDTO = GSON.fromJson(request.body(), MoveRequestDTO.class);
String current = moveRequestDTO.getCurrent();
String destination = moveRequestDTO.getDestination();
String teamType = moveRequestDTO.getTeamType();
chessService.updateHistory(current, destination, teamType, roomId);
BoardDTO boardDTO = findLatestBoardByRoomId(roomId);
return GSON.toJsonTree(boardDTO);
} |
오.. 저는 DTO를 어디까지 사용 할 것인가에 관해 고민해보셨나 해서 남겼는데, 엄청 많이 고민 해보셨네요 👍 우선 결론적으로는 케빈 말씀대로 답은 없는 것 같아요 :)
사실 캐싱 피드백을 남길때 남겨도 될지 고민을 많이 했어요. 우선 제가 생각했던 캐싱은 Board객체가 만들어 졌을때 Map에 저장하고 변화가 있을때 해당 Map을 갱신하는 형태 였습니다. 캐싱은 포기 하셨지만 많은 고민을 하신 것 같아서, 지금은 이만 넘어가도 충분 할 것 같아요 :) |
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.
케빈 안녕하세요!
이번 미션은 마무리해도 될 것 같아요. 고생 많으셨습니다!
질문 주신 부분에 관해 코멘트 남겼어요. 확인 부탁드릴게요.
궁금한점 있다면 편하게 DM주세요!
앗... ㅠㅠㅠ Map을 사용한 캐싱 말씀이셨군요! 계속 DB에 저장해서 사용하는 방식을 찾는데 몰두하다 보니 체스보드 정보 자체를 db에 저장하려고 했었네요 :) 그 방법을 사용하면 move할 때마다 chessboard를 번거롭게 새로 생성하여 초기화하는 작업이 필요없겠네요. |
학습 로그[JDBC] JDBC와 자원 관리 - 5내용
[JS] Ajax - 3내용
[SQL] 테이블 설계 - 4
Command Query Separation - 1Command (tell) : 객체 내부 상태를 변경한다면 결과값을 반환하지 않는 명령만을 수행하는 메서드 |
안녕하세요 데이브.
5단계 미션은 db연동 외에도 방마다 게임을 다르게 실행할 수 있는 선택 요구사항도 구현했어요.
db에 history를 저장하는 방식
웹서버가 종료되더라도 게임 정보가 유지될 수 있도록 다음과 같은 history 테이블을 생성해두었는데요. 게임을 진행하면서 발생한 기물들의 모든 움직임 로그를 담고있습니다.
이 방식을 적용하다보니 매번 ChessService에서
등이 있을 때마다 chessboard를 새로 생성한 다음 history 데이터를 기반으로 해당 chessboard를 일일이 변경
histories.restoreLatestChessBoard(chessBoard);
시킵니다.이후 이를 바탕으로 사용자의 요청을 처리하고 응답 메시지를 작성합니다.
매번 요청이 있을 때마다 새롭게 chessboard를 만들고 이동시키는 작업이 비효율적이라는 생각이 듭니다. 😭 다른 접근 방법이 있을까요..?