[스티치] 체스 4, 5단계 웹 & DB 미션 코드리뷰 제출합니다 #155

lxxjn0 · 2020-04-08T12:38:12Z

웹과 데이터베이스가 이번에 처음이라 부족한 부분이 많을 것 같습니다.
아직 배워야 하는 점이 많습니다.

많은 지적해주시면 감사하겠습니다!!!

jihan805

안녕하세요~ 구현 잘하셨어요!
몇가지 피드백 남겼으니 확인해주세요 !

jihan805 · 2020-04-11T15:52:14Z

src/main/java/chess/service/ChessService.java

+		}
+	}
+
+	private void createChessHistory() throws SQLException {


Service 구현 👍
테이블 생성 여부를 Service에서 관리하는 것이 좋은 방법일까요?

제가 이번에 데이터베이스와 웹을 처음 접하면서 미흡한 점이 많습니다.

처음에는 도커를 통해서 관리하면 되겠구나! 라고 생각했습니다.
그런데 데이터베이스 자체가 영속성을 지니기 때문에 휘발성이란 특성이 존재하는 도커와 조금 반대되는 철학인 것이 아닐까? 하는 글을 보았습니다.

제가 위처럼 작성한 것은 처음 프로그램을 실행한 사용자가 테이블을 만들지 않아도 사용할 수 있도록 편의성을 제공하면 어떨까하고 만들어봤는데 테이블의 생성 여부는 코드에서 관여할 부분이 아닌가요?
기본적으로 사용하는데 필요한 테이블은 이미 만들어져있다는 가정하에 진행하면 되는 것인가요???

jihan805 · 2020-04-11T15:55:26Z

src/main/java/chess/web/dao/JdbcChessHistoryDao.java

+
+	@Override
+	public void deleteAll() throws SQLException {
+		String query = "DELETE FROM " + CHESS_HISTORY_TABLE;


이번 미션에서 db가 main이 아니기에 꼭 반영할 필요는 없지만, 재시작 한다고 테이블의 모든 데이터를 삭제할 필요가 있을까요?
고민해보면 좋을 것 같아요 :)

다른 크루들에게 조언을 구해봤을 때는, 새로운 게임이 시작되면 새로운 테이블을 만들어서 새 게임에 대한 데이터를 다시 저장하는 것이 어떻겠냐는 이야기를 들었습니다.

제가 아직 db에 익숙하지 않아서 테이블을 어떻게 게임마다 만드는 지.
만든 다음 코드와 어떻게 연결지어 사용하는 지 방법을 잘 몰라서 구현하지 못했습니다.
한번 찾아보고 다시 구현해보도록 하겠습니다.

jihan805 · 2020-04-11T15:59:36Z

src/main/java/chess/controller/WebChessController.java

+	}
+
+	private String renderChessBoard(final ChessGame chessGame) {
+		ChessBoardDto chessBoardDto = chessGame.getChessBoardDto();


ChessGame이라는 도메인 객체에서 dto로 변환하는 대신, Dto Converter를 만들어보면 어떨까요?

dto converter라면 dto로 변환만 해주는 새로운 클래스를 말씀하시는 건가요?

여러 자료를 찾아보다가 정적 팩토리 메서드로 dto 생성을 하던데 이처럼 한번 구현해보겠습니다!!

jihan805 · 2020-04-11T16:01:37Z

src/main/java/chess/controller/WebChessController.java

+	}
+
+	private String playChessGame(final Request request, final Response response) {
+		ChessGame chessGame = chessService.loadChessGame();


controller에서 load하고, move를 각각 호출해야 할까요?
특히 moveChessPiece는 서비스에 조금 더 어울릴 것 같아요!

service라는 이름의 클래스도 이번에 처음 만들어봐서 정확히 어떠한 역할과 책임을 가져야 하는지 정확히 알지 못했습니다.

그래서 책임 분리에 미흡한 부분이 있었던 것 같은데 추가적으로 공부를 해서 좀 더 각각의 클래스 이름에 맞게 책임을 분리하도록 진행해보겠습니다!!!

jihan805 · 2020-04-11T16:02:46Z

src/main/java/chess/web/repository/JdbcChessConnection.java

+
+		// 드라이버 연결
+		try {
+			con = DriverManager.getConnection("jdbc:mysql://" + server + "/" + database + option, userName,


를 이용하는 대신 String 에서 제공하는 메서드를 이용하면 어떨까요?

∙ 의 경우는 어떤 부분에 대한 피드백인지 잘 모르겠습니다.
혹시 위의 + 부분을 String.format 을 활용하여 수정하라는 말씀이신가요?

위의 코드를 이전에 테크코스 수업에서 제공해준 코드를 가져와서 활용해 보았는데 혹시 더 괜찮은 방법이 있다면 알려주시면 수정해보겠습니다.

jihan805 · 2020-04-11T16:07:52Z

src/test/java/chess/service/ChessServiceTest.java

+import chess.domain.chessGame.ChessCommand;
+import chess.domain.position.Position;
+import chess.web.dao.ChessHistoryDao;
+import chess.web.dao.InMemoryChessHistoryDao;


dao의 패키지가 web 아래 위치하는 것이 조금 어색하네요!

좀 더 바람직한 위치를 확인해보고 수정하도록 하겠습니다!!

lxxjn0 · 2020-04-16T11:54:53Z

안녕하세요 써머 리뷰어님!
이번에 코드를 정말 대대적인 수정을 했습니다.. ㅜㅡㅜ
이전과 비교해서 정말 많은 부분이 변경됐는데 아래에 간략하게 적어두고 궁금한 점도 함께 적어두겠습니다!!!

domain

도메인은 이전 단계에서 머지하시면서 너무 추상화가 깊다고 말씀해주신 부분에 대해 수정하였습니다.
기존에 ChessPiece -> NonLeapablePiece -> Pawn -> BlackPawn / WhitePawn 처럼 되어 있던 구조를 탈피하였습니다.

그리고 이전에 RuleStrategy를 주입받아서 ChessPiece를 생성하는 형식으로 구현 했었습니다.
이때 말씀해주신 부분이 ChessPiece가 자신의 전략이 아닌 것을 가질 수 있는 문제점이 존재한다고 말씀해주셨습니다.
그래서 1단계로는 그냥 각각의 ChessPiece 객체(Pawn, King과 같은)에 바로 전략을 넣는 형식으로 구현하였습니다.

이처럼 구현하는 경우에 각각의 ChessPiece 객체들을 계속하여 생성해야 한다는 아쉬움이 존재했습니다. 그래서 이번에는 실질적인 규칙을 가지는 PieceStrategy를 구현하고 PieceType이라는 열거형 객체를 사용하여 미리 만들어 두었고, 이를 가져다 쓰는 형식으로 변경해보았습니다.
혹시 한번 보시고 구조가 조금 이상하다면 알려주시면 감사하겠습니다!!!

추가적으로 PieceDirection를 만들고 기존에 PieceState를 조금 간략화 시키는 등의 전체적인 정리도 하였습니다.

database

아무래도 이번 미션에서 가장 많이 어려웠던 부분이었던 것 같습니다.
데이터베이스를 처음 구현해본 만큼 미흡했던 점이 많았던 것 같습니다.

이전에 리뷰어님께서 피드백 주신 부분 중에서

새로운 게임을 시작하는 경우에 기존의 데이터베이스를 모두 지우는 방식말고 더 나은 방식은 없을까요?

라는 부분이 있었습니다.

데이터베이스를 잘 다루지 못해서 하나의 테이블에서 관리를 해보려고 했는데, 시간적인 여유도 있는 만큼 최대한 말씀해주신 부분에 대해 적용해보고자 노력하였습니다.

그래서 전체적인 테이블 구성을 아래와 같이 구성해보았습니다.

CREATE TABLE chess_games (
    game_id bigint NOT NULL AUTO_INCREMENT,
    created_time timestamp NOT NULL,
    PRIMARY KEY (game_id)
)

CREATE TABLE chess_histories (
    history_id bigint NOT NULL AUTO_INCREMENT,
    game_id bigint NOT NULL,
    start varchar(5) NOT NULL,
    end varchar(5) NOT NULL,
    created_time timestamp NOT NULL,
    PRIMARY KEY (history_id)
)

chess_games 테이블에서는 게임에 대한 id를, chess_histories 테이블에서는 각 게임에서 진행한 이동 명령들을 저장하였습니다.
실질적으로 chess_histories 테이블의 game_id는 foreign key이지만, 아직 실력이 부족해서 이 부분까지 관리할 자신이 없어서 일반 컬럼으로 생성했습니다.

그리고 게임을 진행하면서 game_id에 대한 이동을 chess_histories 테이블에 기록하고, findAllByGameId라는 메서드를 통해 게임을 불러올 수 있도록 구현하였습니다.
또, 아직 방 구현을 하지 않아서 findMaxGameId라는 메서드를 통해 가장 최근 방의 game_id를 가져오고, 이 값으로 위의 findAllByGameId를 사용하였습니다.

매번 게임을 로드할 때 마다 findMaxGameId의 쿼리문을 실행하는 점과 저장하고 로딩할 때 마다 기록들을 읽는다는 점이 많이 아쉽지만, 어떻게 해결할 수 있는지에 대한 방법을 잘 몰라서 그대로 두었습니다.

그리고 또 말씀해주신 서비스에 대한 책임 분리에도 신경을 써서 리팩토링 해보았습니다.
dto의 경우에도 domain에서 생성하지 않고 dto의 정적 팩토리 메서드를 통해 생성하여 의존성을 끊었습니다.

이번 미션에서 데이터베이스나 또는 웹에 대한 부분은 정말 많이 부족했던 것 같습니다. 아직 경험이 없고 처음이어서 그런 것 같습니다. 그래도 최대한 할 수 있는 최대치로 구현해보려고 노력해보았습니다.

서비스의 경우도 InMemoryDao를 구현하여 테스트해보았고, 전체적인 테스트 커버리지를 신경쓰면서 테스트와 도메인 모델에 집중하여 리팩토링 해보았습니다.

더 수정해야 할 부분이 많을 것입니다.. ㅜㅡㅜ
아쉽거나 더 고치면 좋을 부분들에 대해서 많은 피드백 남겨주시면 정말 감사하겠습니다!!!

jihan805

안녕하세요~! 구현 & 리팩토링 잘하셨어요 💯
머지할게요, 그동안 수고 많으셨어요 :)

jihan805 · 2020-04-19T13:22:08Z

src/main/java/chess/dao/MySqlChessHistoryDao.java

+	}
+
+	@Override
+	public List<ChessHistoryEntity> findAllByGameId(final long gameId) {


움직인 히스토리 대신, 해당 시점의 체스판의 상태를 저장할수도 있을 것 같아요!
또한 이전에 비해 테이블이 가진 구조가 훨씬 좋네요 👍

jihan805 · 2020-04-19T13:40:06Z

src/main/java/chess/controller/WebChessController.java

+	}
+
+	private String render(final ChessGameDto chessGameDto) {
+		final ChessBoardDto chessBoardDto = chessGameDto.getChessBoardDto();


중복을 제거하여 조금 더 깔끔하게 구현해도 좋을 것 같아요!

jihan805 · 2020-04-19T13:41:53Z

src/main/java/chess/web/repository/ConnectionManager.java

@@ -0,0 +1,9 @@
+package chess.web.repository;


web아래 repostiory라는 패키지에 들어있는 클래스들이 조금 어색하네요!!
패키지 정리에 대해서도 고민해보면 좋을 것 같아요!

lxxjn0 added 10 commits April 4, 2020 14:34

[feat] chessboard 기본 css 구현

3fa6541

feat : chess game 시작 페이지 구현

f79422c

[feat] web을 위한 DTO 구현

80d065c

[feat] chessCommand 입력 영역 생성

867125f

[feat] chessGame 웹 구현

2b0f44f

[feat] PieceColorDto 구현

69fdaeb

[feat] JdbcConnection 구현

3af21a4

[feat] chess_db 적용

3d041d6

[refactor] WebChessPiece 명명 수정

53b0f60

[refactor] Dao 테스트 코드 작성

b8a3184

jihan805 suggested changes Apr 11, 2020

View reviewed changes

lxxjn0 added 2 commits April 16, 2020 20:28

[refactor] 피드백 반영

3e2e4bc

[refactor] 피드백 반영

578ab34

jihan805 approved these changes Apr 19, 2020

View reviewed changes

jihan805 merged commit 3eaebe7 into woowacourse:lxxjn0 Apr 19, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[스티치] 체스 4, 5단계 웹 & DB 미션 코드리뷰 제출합니다 #155

[스티치] 체스 4, 5단계 웹 & DB 미션 코드리뷰 제출합니다 #155

lxxjn0 commented Apr 8, 2020

jihan805 left a comment

jihan805 Apr 11, 2020

lxxjn0 Apr 14, 2020 •

edited

Loading

jihan805 Apr 11, 2020

lxxjn0 Apr 14, 2020

jihan805 Apr 11, 2020

lxxjn0 Apr 14, 2020 •

edited

Loading

jihan805 Apr 11, 2020

lxxjn0 Apr 14, 2020

jihan805 Apr 11, 2020

lxxjn0 Apr 14, 2020

jihan805 Apr 11, 2020

lxxjn0 Apr 14, 2020

lxxjn0 commented Apr 16, 2020

jihan805 left a comment

jihan805 Apr 19, 2020

jihan805 Apr 19, 2020

jihan805 Apr 19, 2020

[스티치] 체스 4, 5단계 웹 & DB 미션 코드리뷰 제출합니다 #155

[스티치] 체스 4, 5단계 웹 & DB 미션 코드리뷰 제출합니다 #155

Conversation

lxxjn0 commented Apr 8, 2020

jihan805 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lxxjn0 Apr 14, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lxxjn0 Apr 14, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lxxjn0 commented Apr 16, 2020

domain

database

jihan805 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lxxjn0 Apr 14, 2020 •

edited

Loading

lxxjn0 Apr 14, 2020 •

edited

Loading