Skip to content
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

Merged
merged 54 commits into from
Apr 5, 2021

Conversation

xlffm3
Copy link

@xlffm3 xlffm3 commented Mar 30, 2021

안녕하세요 데이브.

5단계 미션은 db연동 외에도 방마다 게임을 다르게 실행할 수 있는 선택 요구사항도 구현했어요.

db에 history를 저장하는 방식

웹서버가 종료되더라도 게임 정보가 유지될 수 있도록 다음과 같은 history 테이블을 생성해두었는데요. 게임을 진행하면서 발생한 기물들의 모든 움직임 로그를 담고있습니다.

CREATE TABLE History (
	ID INT NOT NULL AUTO_INCREMENT,
	SOURCE VARCHAR(255),
	DESTINATION VARCHAR(255),
	TEAM_TYPE VARCHAR(255)
	ROOM_ID INT NOT NULL,
	
	PRIMARY KEY (ID),
	CONSTRAINT ROOM_FK FOREIGN KEY(ROOM_ID) REFERENCES ROOM(ID)
);
CREATE TABLE Room (
	ID INT NOT NULL AUTO_INCREMENT,
	NAME VARCHAR(255),

	PRIMARY KEY (ID)
);

이 방식을 적용하다보니 매번 ChessService에서

  1. 체스보드 정보 요청
  2. 이동 요청

등이 있을 때마다 chessboard를 새로 생성한 다음 history 데이터를 기반으로 해당 chessboard를 일일이 변경 histories.restoreLatestChessBoard(chessBoard); 시킵니다.
이후 이를 바탕으로 사용자의 요청을 처리하고 응답 메시지를 작성합니다.

매번 요청이 있을 때마다 새롭게 chessboard를 만들고 이동시키는 작업이 비효율적이라는 생각이 듭니다. 😭 다른 접근 방법이 있을까요..?

xlffm3 added 30 commits March 29, 2021 14:30
Copy link

@dave915 dave915 left a 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 {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DTO가 Service내부에서 쓰이는 것에 대해 어떻게 생각하시나요? 🤔

Copy link
Author

@xlffm3 xlffm3 Apr 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

음... 이걸 사실 고민을 많이했어요 데이브.

  • 서비스 레이어에서 parameter로 dto를 받아도 될까?
  • 서비스 레이어는 dto를 반환해도 될까?

DTO는 어느 레이어까지 사용하는 것이 맞을까?

여러 글을 참조했는데요. 파라미터로 받는건 몰라도 반환하는 것에 대해서는 다시금 생각해볼 여지가 있을 것 같아요.

Comment on lines 40 to 42
ChessBoard chessBoard = generateDefaultChessBoard();
Histories histories = new Histories(chessRepository.findAllHistoriesByRoomId(roomId));
histories.restoreChessBoardAsLatest(chessBoard);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

매번 초기화 하는 부분에 대해 질문 주셨는데요!
ChessBoard의 현재 캐싱하고 있다가, ChessBoard에 변화가 있을때만 갱신해주는 방법은 어떠신가요?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

쉽지 않겠지만 방법을 강구해봐야겠습니다. ㅎㅎ

Comment on lines 25 to 26
response.type(RESPONSE_JSON);
int roomId = Integer.parseInt(request.params(":roomId"));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

중복되는 부분은 메소드로 분리 해도 좋을 것 같아요!

@xlffm3
Copy link
Author

xlffm3 commented Apr 2, 2021

안녕하세요, 데이브 :)

중복되는 메서드 분리

ChessController에서 중복되는 부분들을 메서드로 분리했어요.

ChessController.java

    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가 서비스 레이어 내부에서 쓰이는 것에 관하여

  • DTO를 어디까지 사용할까?
  • DTO-Entity 간의 변환 위치는 어디가 되어야 할까?

라는 질문에 대해

https://www.slipp.net/questions/93
https://stackoverflow.com/questions/21554977/should-services-always-return-dtos-or-can-they-also-return-domain-models
https://buildplease.com/pages/repositories-dto/
https://www.baeldung.com/entity-to-and-from-dto-for-a-java-spring-application

여러 아티클들을 참고했습니다. 그리고 데이브가 준 질문에 대해 '정답은 없다'고 생각했어요. (실무를 모르는 입장에서 제가 접근할 수 있는 자료들을 통해 내린 짧은 생각입니다! ㅎㅎ 😔)

Service Layer가 Domain 모델을 반환할지 DTO를 반환할지는 상황에 맞게 취사선택하라는게 여러 글들의 요지였는데요.

  • Controller(Presentation)와 Service Layer간 메시지를 주고받을 때 엄격하게 DTO를 사용해야 하는가?
    • 컨트롤러에서 Domain 모델을 사용하게 된다면 코드 결합도가 증가하여 추후 유지보수 이슈가 발생할 수 있다. (👀)
    • 서비스 레이어가 Domain 레이어를 캡슐화해 보호한다.
  • 그러나 도메인 모델과 유사한 DTO를 사용하면 코드 복제 및 비용의 문제가 발생한다.
    • 큰 규모의 프로젝트에서는 DTO 사용이 권장되지만, 작은 규모는 불필요할수도 있다.
  • 따라서 서비스 레이어의 반환 타입은 코드의 복잡성 vs 도메인 모델의 보호와 안전성, Presentation layer와의 결합력 감소 등을 판단해서 선택하라.

제 기존 코드들은 공통적으로 Service Layer에서 DTO를 받고 DTO를 반환했습니다.

DTO를 Domain으로, Domain을 DTO로 변환하는 위치를 꼭 컨트롤러가 아닌 서비스 레이어에서 해도 되지 않을까? 하고 조심스럽게 생각했어요. DTO는 레이어간 데이터 전달의 목적이기때문에 어디서 해도 크게 문제가 되지 않을 것 같고, 위 글들에 따르면 서비스에서 도메인 모델을 보호하는 겸 겸사겸사...? 😂

이번에 리뷰 반영할 때는 서비스 레이어들이 DTO가 아닌 도메인 모델을 반환하도록 코드를 수정했습니다. 두 방식을 모두 사용해보았지만 현재 제 수준에서 쉽게 결론을 지을 주제가 아니라고 느꼈습니다.

다만 DTO 반환이 아닌 도메인 반환을 통해, 비대했던 Service Layer의 메서드들이 간결해지고 한 번에 한 가지 일만 수행하도록 더욱 분리할 수 있었습니다. 😂😂 데이브는 어떤 생각을 하시는지 알려주신다면 제가 공부하는데 큰 도움이 될것같아요!


ChessBoard 캐싱에 관해

현재 1, 2, 3, .... k개의 방이 있으면 각 방마다 개별적인 체스 게임 플레이가 가능하도록 되어 있는데요.
리뷰어님께서 말씀해주신 캐싱을 적용해보려고 했는데 여러 문제로 적용을 포기했습니다.

  • ChessBoard는 좌표값을 key, 보드의 1칸(Cell)을 value로 가지고 있습니다. 총 64칸의 cell을 보유합니다.
  • 각각의 cell은 가변 필드로 piece를 가지고 piece가 null이면 비어있는 칸임을 의미합니다.
  • k개의 방이 있다면 k개의 체스보드들은 서로 각각 다른 cell을 가져야하기 때문에 캐싱하기가 어렵다고 판단했습니다.
  • HIstory Table에 의존하는 현재 방식이 아닌 ChessBoard Table을 만들고 각 방에서 사용하는 보드 객체 자체를 DB에 저장하려고 했으나..
    • chessboard 객체 내부에는 다양한 레퍼런스 필드들이 존재하는데,,, 객체 자체를 저장하려면 테이블 연관관계 등 고려할 사항이 많으나 현재 제 지식으로는 이를 만드는것이 어렵다고 판단했습니다.
    • 직렬화, 리플렉션 등을 통해 객체의 상태를 저장할 수 있겠으나 두 방식은 보안 등 여러 단점이 존재한다고 판단해 지양했습니다.

대신 기존 ChessService의 코드에 중복이 심하다고 판단해서 중복되는 부분을 별도의 메서드로 분리했습니다. 또한 일전의 Service Layer는 DTO를 반환하다 보니 하나의 메서드가 여러 동작을 수행했었는데요.

예를 들어, 기존의 BoardDTO move(MoveRequestDTO request, int roomId) 메서드는

  • History이력을 업데이트하고,
  • View에서 사용할 Board 객체를 찾아 DTO로 변환해 반환.

등 두 가지 일을 했습니다.

메서드는 한 가지 일만 잘 해야한다는 생각이 들었고, CQS라는 개념을 접하게 되었는데요.

Command Query Separation

  • Command (tell) : 객체 내부 상태를 변경한다면 결과값을 반환하지 않는 명령만을 수행하는 메서드
  • Query (ask) : 객체 내부 상태 변경없이 결과값만 제공하는 질의 메서드

기존 ChessService 메서드들은 CQS를 어기고 있었고, 히스토리 이력만 업데이트하는 명령 메서드로 리팩토링했습니다.

public void updateHistory(String current, String destination, String teamType, int roomId)

이후 컨트롤러에서 조회 쿼리 메서드를 통해 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);
    }

@dave915
Copy link

dave915 commented Apr 5, 2021

DTO가 서비스 레이어 내부에서 쓰이는 것에 관하여

오.. 저는 DTO를 어디까지 사용 할 것인가에 관해 고민해보셨나 해서 남겼는데, 엄청 많이 고민 해보셨네요 👍

우선 결론적으로는 케빈 말씀대로 답은 없는 것 같아요 :)
저는 원칙적으로는 Service레이어에 DTO가 들어오면 안된다고 생각합니다. 그래야 여러 종류의 컨트롤러에서 해당 서비스를 사용 할 수 있거든요.
하지만 현업을 하다보면 여러 종류의 컨트롤러가 한 서비스를 사용하기 보단, 한 종류의 컨트롤러가 서비스를 쓰기 때문에 너무 그렇게 엄격히 제안할 필요가 있냐는 의견도 있어요. 그래서 보통 코드를 보면 서비스로 DTO 진입을 허용하되 서비스 메소드 상위에서 DTO 체크 및 도메인 변환을 하고 변환 후에는 DTO는 사용하지 않고 도메인만 사용하도록 구현 되어있어요. (DTO가 서비스로 들어올 수 있기 때문에 초기에 도메인으로 변환한다는 규칙을 지키지 않는다면... DTO가 Repository까지 진입하는 경우도 있습니다😭 )
반환 타입 같은 경우에는 보통 도메인 객체를 내보내주고 컨트롤러에서 DTO로 변환하고 있습니다 :)

ChessBoard 캐싱에 관해

사실 캐싱 피드백을 남길때 남겨도 될지 고민을 많이 했어요. 우선 제가 생각했던 캐싱은 Board객체가 만들어 졌을때 Map에 저장하고 변화가 있을때 해당 Map을 갱신하는 형태 였습니다.
물론, Web은 멀티 스레드로 움직이기때문에 동기화도 신경써야하고, 여러 인스턴스로 구동되었을땐 내부 Map이 아닌 redis 같은 외부 캐시서버를 사용하는 등 많은 고려가 필요합니다.

캐싱은 포기 하셨지만 많은 고민을 하신 것 같아서, 지금은 이만 넘어가도 충분 할 것 같아요 :)

Copy link

@dave915 dave915 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

케빈 안녕하세요!
이번 미션은 마무리해도 될 것 같아요. 고생 많으셨습니다!
질문 주신 부분에 관해 코멘트 남겼어요. 확인 부탁드릴게요.
궁금한점 있다면 편하게 DM주세요!

@dave915 dave915 merged commit f1ae4c0 into woowacourse:xlffm3 Apr 5, 2021
@xlffm3
Copy link
Author

xlffm3 commented Apr 5, 2021

앗... ㅠㅠㅠ Map을 사용한 캐싱 말씀이셨군요! 계속 DB에 저장해서 사용하는 방식을 찾는데 몰두하다 보니 체스보드 정보 자체를 db에 저장하려고 했었네요 :) 그 방법을 사용하면 move할 때마다 chessboard를 번거롭게 새로 생성하여 초기화하는 작업이 필요없겠네요.

@xlffm3
Copy link
Author

xlffm3 commented Apr 13, 2021

학습 로그


[JDBC] JDBC와 자원 관리 - 5

내용

  • JDBC Driver를 로드하고 Connection 객체 생성 뒤, Statement 객체를 생성하고 이후 쿼리를 실행하여 원하는 동작을 수행한다.
  • ResultSet, Statement, Connection등 할당된 자원은 모두 close해주어야 한다.
    • try-with-resources를 사용하면 선언된 자원의 AutoCloseable 인터페이스에 의해 자동 close된다.
  • Statement는 쿼리를 분석하고 컴파일한 뒤 실행하는데, PreparedStatement는 첫 1회만 분석 및 컴파일하고 그 이후 캐시에 담아 사용하기 때문에 반복 사용에 있어서 유리하다.

[JS] Ajax - 3

내용

  • xmlHttpRequest 및 비동기 방식을 통해 화면을 새로고침 없이 원소들을 수정할 수 있다.
    • 최근에는 axios 방식을 더 이용하는 듯 하다. Promise 등의 개념을 이해해서 js-todo-list-step2 미션에서 적용해야겠다.
  • eventListener와 함께 사용함으로써 체스 기물 엘리먼트를 클릭시 없앤다.

[SQL] 테이블 설계 - 4

  • 외래키 제약을 통해 두 테이블간의 연관관계를 만들고 효과적으로 원하는 기능을 구현할 수 있다.
  • 이번에는 History를 매 기물 이동마다 저장하고, 새로 게임을 실행할 때 마다 체스보드를 히스토리에 의해 업데이트시켜야 한다는 단점이 존재한다.
    • 체스보드 객체를 JSONString으로 변환해 저장해두고 (GSON 혹은 ObjectMapper 등으로) 객체간 변환하는 방식도 고려해보자.
    • 다른 직렬화을 쓸 바엔 JSON! - effective java

Command Query Separation - 1

Command (tell) : 객체 내부 상태를 변경한다면 결과값을 반환하지 않는 명령만을 수행하는 메서드
Query (ask) : 객체 내부 상태 변경없이 결과값만 제공하는 질의 메서드

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants