[3, 4단계 - 체스] 허브(방대의) 미션 제출합니다. #529

greeng00se · 2023-03-21T08:38:15Z

안녕하세요 찰리 🍫 오랜만입니다.
잘 지내셨나요?

4단계의 필수 요구사항까지 일단 적용해보았습니다.

데이터베이스 테이블에 대한 정보는 어떻게 남겨야 할까요?
일단 쿼리문으로 남겨놓도록 하겠습니다!

CREATE TABLE move
(
    id     INT        NOT NULL AUTO_INCREMENT PRIMARY KEY,
    source VARCHAR(2) NOT NULL,
    target VARCHAR(2) NOT NULL
);

사용하는 테이블은 위와 같습니다.

변경 사항은 다음과 같습니다.

board에 상태 패턴 적용
점수 계산 기능 구현
왕이 잡히면 게임 종료 기능 구현(정확히는 보드의 상태가 end가 됩니다.)
jdbc 템플릿 콜백 패턴(이 맞는지는 모르겠지만 비슷하게 적용해보았습니다.) 적용

등등.. 작은 수정들도 같이 변경했습니다!

추가로 Controller의 필드로 가지는것이 아닌 지역변수로 관리해보면 어떨까요?
남겨주신 위 코멘트에 대해서는 4단계 선택 요구사항을 구현할 때 반영해보겠습니다.

감사합니다!

Gomding

안녕하세요 허브!
3,4 단계 빠르게 진행해주셨네요 🙇
몇가지 코멘트 남겼으니 확인부탁드려요~!!

테이블 정보는 코멘트로 남겨주셔도 되고, chess.sql 처럼 파일로 생성했다고 알려주시면 됩니다 😄

Gomding · 2023-03-22T06:16:53Z

src/main/java/chess/repository/ConnectionGenerator.java

+        } catch (final SQLException e) {
+            System.err.println("DB 연결 오류:" + e.getMessage());
+            e.printStackTrace();
+            return null;
+        }


SQLException 예외를 catch 하여 출력만 하고 끝내면 외부에서는 예외처리가 불가능하겠네요 😭
예외가 발생했음에도 getConnection을 호출한 곳에서는 로직이 정상 수행되고 있어요

현재는 null을 리턴하기에 어디선가 Connection을 쓸 때 NullPointerException 예외가 발생할 수 있겠네요~
어디서 NullPointerException이 발생할지 모르기 때문에 더 큰 문제가 될 수 있어요

예외처리와 사라진 SQL Exception

기존 코드를 가져다 사용하는 과정에서 꼼꼼하게 코딩을 하지 못한 것 같습니다!
IllegalStateException으로 예외 전환해서 예외를 던져보도록 하겠습니다!

추가로 IllegalStateException 같은 공통적으로 쓰이는 UncheckedException 과 CustomException 을 사용하는것
둘 중 어떤 것이 더 나을까요?

IllegalStateException 의 경우 원치않게 상위 레벨에서 catch에서 잡힐 수 있다고 생각해요

커넥션을 맺을 때 실패하는 경우는 두 가지 경우가 있을 것같은데요

네트워크 불안정

DB 접근 정보 오류

1번의 경우 retry 로 재접속을 시도는 해볼 수 있지만
2번의 경우는 DB 접근을 위한 정보의 오류로 애플리케이션 자체에서 복구는 불가능할 것같아요!

이럴 때는 원치않게 catch 로 처리되는것보다 예외를 던져주고 빠르게 애플리케이션을 종료시켜야한다고 생각해요.
허브는 어떻게 생각하시나요? 🤔

DatabaseConnectionFailException를 던지고 그냥 애플리케이션이 종료되도록 구현했습니다!
데이터베이스를 연결하지 못하는 경우 오프라인 모드는 어려울 것 같고 바로 종료하는게 좋은 것 같습니다!

Gomding · 2023-03-22T06:17:51Z

src/main/java/chess/repository/JdbcTemplate.java

+        } catch (SQLException ex) {
+            throw new RuntimeException(ex);
+        }


SQLException 을 RuntimeException으로 변환하는 이유가 궁금해요!

위와 같은 이유입니다! 이 또한 적절한 예외로 전환하도록 하겠습니다 😄

Gomding · 2023-03-22T20:13:36Z

src/main/java/chess/repository/JdbcTemplate.java

+        }
+    }
+
+    public <T> T query(final String query, final RowMapper<T> rowMapper) {


템플릿/콜백 패턴 이용 👍

Gomding · 2023-03-22T20:25:30Z

src/main/java/chess/controller/ChessGameController.java

 import chess.view.InputView;
 import chess.view.OutputView;
 import java.util.EnumMap;
 import java.util.List;
 import java.util.Map;

 public class ChessGameController {
-
    private final ChessGame chessGame;


1단계 코멘트 댓글로 남겨주신 부분에 대한 답변입니다!

오.. 동시에 여러 게임이 진행된다면 static 필드로 가지고 있는게 좋은 것 같습니다! 여러 컨트롤러가 ChessGame을 공유할 수 있겠군요! 감사합니다 😄

동시에 여러 게임이 진행된다면 Controller 는 상태를 가지지 않는게 좋을것같아요 🤔
만약 여러 컨트롤러가 ChessGame을 공유한다면 다 함께 같은 게임을 바라보지 않을까요?

여러 게임이 진행된다면 Controller 는 하나만 존재하게 하고
Controller 는 특정 ChessGame에 연결하는 역할을 하는게 좋겠어요

오 매우 좋은 것 같습니다!
저는 ChessGame이 Map 형태로 여러 개의 보드를 가지고 있는 것을 생각해서, 위와 같이 답변을 했습니다! 추후에 적용할 부분에 대해 조금 더 많은 설명을 남기지 못해 아쉬움이 많이 남는 것 같습니다 😢
ChessGame을 각각 내부에서 생성하는 방법도 좋을 것 같다는 생각이 듭니다! 따라서 코멘트 남겨주신 구조대로라면 ChessGame이 도메인이 다시 위치하는게 좋을 것 같습니다!

Gomding · 2023-03-22T20:33:47Z

src/main/java/chess/controller/ChessGameController.java

    private final ChessGame chessGame;
    private final Map<Command, ChessGameAction> commandMapper = new EnumMap<>(Command.class);

    public ChessGameController(final ChessGame chessGame) {
        this.chessGame = chessGame;
-        commandMapper.put(START, this::start);
+        commandMapper.put(START, ignore -> start());


람다 매개변수 ignore 가 의미하는것이 무엇인지 궁금해요!

입력을 무시한다는 뜻으로 사용을 해보았습니다!

ignore로 의도가 전달될지 의문이 들었지만 이런 컨벤션만 있으면 나중에 이해할 수 있다고 생각이 드네요!! (람다 매개변수가 ignore 일 때는 사용하지 않음)
저는 commands 를 매개변수에 넣지 않으면 이해되는 부분으로 보여요!

Suggested change

commandMapper.put(START, ignore -> start());

commandMapper.put(START, commands -> start());

commandMapper.put(MOVE, commands -> move(commands));

무엇이 더 나은지는 허브가 판단하시면 됩니다 ㅎㅎ

Gomding · 2023-03-22T21:11:24Z

src/main/java/chess/domain/board/AbstractBoard.java

+import chess.domain.position.Position;
+import java.util.Map;
+
+public abstract class AbstractBoard implements Board {


Board 라는 interface 가 꼭 존재해야할까요 🤔

현재 AbstractBoard와 Board의 차이는 필드의 유무밖에 없어보여요!
어떤 이유로 한 계층을 더 만들었는지가 궁금합니다!

인터페이스는 해당 보드에 대한 타입을 정의하는 용도로 사용하고, AbstractBoard는 필드와 하위 구현체의 공통 메서드를 가지도록 했습니다.
요구사항에 따라 구현하고 나니 getBoard 부분이 getResult로 변경되면서 필드의 유무 차이 밖에 없어진 것 같습니다!
이런 경우 제거하고 깔끔하게 추상 클래스만 사용하는게 좋을까요?

허브가 생각하시기에 interface가 있는것이 큰 장점이 없다면 제거하셔도 될 것같아요 😃

Gomding · 2023-03-22T21:15:18Z

src/main/java/chess/service/ChessGame.java

@@ -0,0 +1,53 @@
+package chess.service;


제가 생각할 때 ChessGame 은 domain 의 역할을 하고있다고 생각해요!

service 라는 패키지로 변경하신 이유가 있을까요?

하나의 Board를 가지는 경우 ChessGame을 도메인으로 두려고 했습니다!
4단계의 추가 요구사항을 구현할 때 ChessGame이 <Room, Map> 형태로 여러 개의 보드를 가지고 있는 것을 생각했고, 위와 같이 service 계층으로 옮겼습니다.

찰리의 코멘트 남겨주신 방법이 큰 변경없이 적용할 수 있을 것 같아, ChessGame을 컨트롤러 내부에서 생성하고 현재와 같이 Board 하나를 가지고 있는 방법을 선택하여 추가 요구사항을 구현해보도록 하겠습니다!
따라서 다시 domain에 위치하는게 적절한 것 같습니다!

Gomding · 2023-03-22T21:15:59Z

src/main/java/chess/service/ChessGame.java

+        final List<MoveDto> moves = chessDao.findAll();
+        for (MoveDto move : moves) {
+            final Position source = Position.from(move.getSource());
+            final Position target = Position.from(move.getTarget());
+            board = board.move(source, target);
+        }


move 이력을 가져와서 보드 상태를 변경해주기 👍

크게 생각하면 두 방법이 있을텐데요
DB 에 board 전체를 저장하기 VS move 이력을 저장해두고 게임을 불러올 때 이력대로 이동시키기
어떤 측면에서 move 이력만을 저장하기로 하셨나요?

도메인의 변경이 가장 적고, 간단한 방법으로 하려다 보니 체스 기보를 저장하는 쪽으로 선택한 것 같습니다.

board 전체를 저장하는 경우 다음의 단점이 있을 것 같습니다.

턴과 같은 부가적인 요소를 저장해야 합니다. (기보 저장보다 고려해야할 요소 많음)

이동을 할 때 기물이 잡히는 경우 update 쿼리(이동 기물)와 delete(잡힌 기물) 2개의 쿼리를 날려야 합니다.

기보의 경우 매 번 하나의 수만 저장해야된다는 장점이 있을 것 같습니다!

현재 구조에서 도메인의 변경이 크게(초기 상태를 구성하는 부분) 일어나야 합니다.

정리하면 다음과 같습니다.
보드저장: 초기상태에서 32개의 Insert 쿼리(기물의 위치) + 기물 이동 시 움직임 변경(잡히는 경우 2개)
기보저장: 초기상태 애플리케이션에서 구성 + 저장된 기보를 select 쿼리로 조회해서 사용(1회) + insert 쿼리(이동 당 1회)

따라서 기보를 저장하면 더 간단하고, 데이터베이스와 연결을 최소화할 수 있다고 생각해서 기보를 저장하도록 했습니다.

Gomding · 2023-03-22T21:31:52Z

src/main/java/chess/repository/ConnectionGenerator.java

+
+    public Connection getConnection() {
+        try {
+            return DriverManager.getConnection("jdbc:mysql://" + SERVER + "/" + DATABASE + OPTION, USERNAME, PASSWORD);


DB에 접속하고 SQL 을 실행하는 과정에서 가장 리소스가 큰 것은 Connection을 맺는것 이라고 해요!
(SQL 에 따라 다를 수 있겠지만 일반적으로 Connection을 맺는 리소스가 크다는것을 의미합니다!)

현재는 findAll, delete, insert 같은 작업이 일어날 때마다 connection을 생성하고 있어요!
단일 사용자에게 제공하는 서비스이니 Connection을 하나만 만들어놓고 계속 사용하는건 어떨까요?

만약 100만의 요청이 오면 100만번 Connection을 생성해줘야하니 그만큼 힘든 작업이 되겠죠 :)
이를 위해서 connection pool 같은것을 활용하여 미리 connection을 여러개 생성해두고 사용하는 방법이 있습니다.

위 connection pool 링크에 있는 글 중 일부분 입니다!

아래 내용은 MySQL 8.0 기준으로 INSERT문을 수행하는 과정을 나타냅니다. 괄호 안의 숫자는 각 과정을 수행하는 데 필요한 비용의 비율을 의미합니다. 자세하게 확인하고 싶으신 분들은 아래 링크를 통해 참고 부탁드립니다.

MySQL 8.0 Documentation : https://dev.mysql.com/doc/refman/8.0/en/insert-optimization.html
Connecting: (3)
Sending query to server: (2)
Parsing query: (2)
Inserting row: (1)
Inserting index: (1)
Closing: (1)

가장 비용이 많이 드는 작업이 보이시나요?

바로 서버가 DB 접속하기 위해서 Connection을 생성하는 작업이 가장 큰 비용을 차지합니다. 즉, Connection을 반복적으로 생성하는 것은 그만큼 큰 비용이 드는 작업임을 알 수 있습니다.

남겨주신대로 단일 사용자를 위한 서비스니 Connection 하나를 만들어두고 계속 사용해보겠습니다!

Gomding · 2023-03-22T21:35:16Z

src/main/resources/chess.sql

@@ -0,0 +1,6 @@
+CREATE TABLE move


sql 파일로 만들어 주셨군요 👍 👍 👍 👍

# Conflicts: # src/main/java/chess/view/input/RoomInputView.java

greeng00se · 2023-03-26T15:30:52Z

안녕하세요 찰리🍫 주말 잘 보내셨나요?

주말에 시간 널널할 때 보내려했는데 미루다 보니 월요일에 보내게 되었네요 😢

다음 추가 요구사항도 같이 구현해서 리뷰 요청을 보냅니다.

체스 게임방을 만들고 체스 게임방에 입장할 수 있는 기능을 추가한다.
사용자별로 체스 게임 기록을 관리할 수 있다.

2번은 결과를 관리하기 보다는 그냥 생성한 게임의 목록을 반환하는 느낌으로 구현했습니다.

컨트롤러의 구조는 아래와 같이 되어있습니다.

DB는 다음과 같습니다.

변경사항 + 기능은 다음과 같습니다.

멀티스레드 환경에서 동작할 수 있도록 구현

스레드 별로 controller.run() 실행할 수 있도록!
일단 SingleThreadExecutor 적용해놨습니다!
ControllerFactory에서 컨트롤러를 싱글턴으로 생성

1게임 당 1명만 진행 가능(2명이 들어와서 하도록 구현하는건 어려울 것 같았습니다.)
DTO 사용 최소화(계층간 데이터 전달 시 사용하려고 했는데, 코드 양이 많아져서, 값을 그냥 보내는 방향으로 작성했습니다.)
스레드 별 계정 정보 + 게임 정보는 ThreadLocal에 저장
작성하지 못했던 db 테스트 추가 (testchess 데이터베이스를 생성해야 합니다. sql 파일에 넣어놨습니다!)
방 생성 기능 추가
사용자 방 관리 기능 추가
Connection을 미리 생성해놓고 사용하도록 변경

마지막 미션이니 만큼 도메인 변경을 최소화하면서 하고 싶은거 다 해봤습니다!
현재 구조에서 멀티스레드로 여러 명의 사용자가 사용할 때 문제가 발생할 지 궁금합니다!

추가로 ChessGame을 Controller에서 생성하려고 했지만 추가 요구사항 반영하다보니 ChessService으로 이름을 변경하고 여러 개의 보드를 가지고 있도록 구현했습니다.

코드 양이 많아졌습니다. 복잡도가 증가해서 리뷰해주시는데 시간이 많이 들까봐 걱정입니다. 😢
그리고 항상 꼼꼼하게 리뷰 남겨주셔서 감사합니다! 👍

greeng00se · 2023-03-26T16:25:53Z

src/main/java/chess/view/input/RoomInputView.java

+public class RoomInputView {
+    private static final String DELIMITER = " ";
+    private static final int LIMIT = -1;
+
+    private final Scanner scanner;
+
+    public RoomInputView(final Scanner scanner) {
+        this.scanner = scanner;
+    }
+
+    public List<String> readCommand(final String user, final String room) {
+        System.out.println("[로그인 한 유저: " + user + ", 선택한 방 이름: " + room + "]");
+        System.out.println("> 방 조회 : history");
+        System.out.println("> 방 생성 : create 방이름 - 예) create room1");
+        System.out.println("> 방 참가 : join 방번호 - 예) join 1");
+        System.out.println("> 메인 화면 : end");
+
+        final String input = scanner.nextLine();
+
+        return Arrays.stream(input.split(DELIMITER, LIMIT))
+                .map(String::trim)
+                .collect(toList());
+    }
+}


중복되는 부분이 많아서 InputView를 하나만 두고 OutputView 정보를 출력할 수 있을 것 같습니다.. 😢

Gomding

안녕하세요 허브!
추가 미션까지 꼼꼼하게 구현해주셨네요 😄
몇가지 코멘트 남겼으니 확인 부탁드려요!!

궁금한 점은 언제든 편하게 DM 주세요~ 🙏

Gomding · 2023-03-27T22:23:24Z

README.md

+    MainController --> UserController --> UserService --> UserDao
+    MainController --> RoomController --> RoomService --> RoomDao
+    MainController --> GameController --> GameSerivce --> GameDao


리팩토링 시 README 도 계속 관리해주시는군요 👍 👍 👍

Gomding · 2023-03-27T22:30:09Z

src/main/java/chess/view/input/UserInputView.java

+        System.out.println("> 회원가입 : register 이름 - 예) register charlie");
+        System.out.println("> 로그인 : login 이름 - 예) login charlie");


예시를 보고 감동받지 않을 수 없네요 🥺

Gomding · 2023-03-28T00:05:36Z

src/main/java/chess/controller/main/MainController.java

+import chess.view.input.MainInputView;
+import chess.view.output.MainOutputView;
+
+public class MainController implements Controller {


현재 구조는 MainController 는 UserController, RoomController, GameController 를 호출하는것으로 보여요

MainController 까지 Controller 를 구현하는것이 어떤이유일까 의문이 들었습니다 🤔

MainController의 readCommand 메서드의
final Controller controller = commandMapper.getValue(mainCommand); 부분에서 Controller 의 구현체들을 살펴볼 때 MainController 가 있어서 MainController 도 다시 호출되는건가? 라는 생각을 했어요

오.. 코드를 처음 본 사람이라면 오해의 여지가 생길 수 있는 부분이었군요! 생각없이 Controller를 구현하도록 한 것 같습니다!
제가 생각해도 왜 구현했을까? 라는 생각이 드는 것 같습니다 제거하겠습니다 😄

추가로 Controller 라는 이름이 어떤 정보를 줄 수 있을까? 라는 의문도 들었습니다~!! 🤔

SubController는 어떨까요? 마땅한 네이밍이 생각나지 않습니다!

Gomding · 2023-03-28T00:11:57Z

src/main/resources/chess.sql

+(
+    id       int PRIMARY KEY AUTO_INCREMENT,
+    name     varchar(255),
+    user_id  int


외래키(foreign key)를 추가해보면 어떨까요?

데이터의 무결성을 위해 외래키를 설정해주는 것이 더 좋은 것 같습니다! 👍
지금 저의 dao테스트의 경우 테스트용 테이블에 추가하는 경우 하나의 dao 테스트에 다른 dao도 필요할 것 같아보입니다.
테스트용 테이블에는 추가를 하지 않아도 괜찮을까요?

테스트는 최대한 실제 환경과 동일하게 맞춰주는것이 예외 케이스를 걸러줄 수 있을거라 생각해요! 😉

Gomding · 2023-03-28T00:23:33Z

src/main/java/chess/repository/UserJdbcDao.java

+    public User findByName(final String name) {
+        return jdbcTemplate.query("SELECT * FROM user WHERE name = ?", resultSet -> {
+            if (resultSet.next()) {
+                final int id = resultSet.getInt("id");
+                final String findName = resultSet.getString("name");
+                return new User(id, findName);
+            }
+            return null;
+        }, name);


Java 에서는 메서드 반환값이 null일 수 있다는 것을 항상 염두에 두고 개발을 해야하는데요
이를 개선하고자 null-safety 하게 만드는 Optional 이라는것이 있습니다.

메서드 반환 타입이 Optional 라면 메서드를 호출하는 쪽에 값이 있을수도 없을수도 있으니 처리는 직접하세요~ 라는것을 알려줄 수 있습니다.

아래 아티클로 Optional 을 학습해보죠!
Optional 클래스
Java Optional 바르게 쓰기

null을 사용하는 것보다 Optional을 사용하여 반환값이 없다는 것을 명시적으로 표현해주는 것이 더 좋아보입니다!
단일 조회의 경우 Optional을 사용하고, 다중 조회의 경우 빈 리스트를 반환하도록 하겠습니다!! 😄

Gomding · 2023-03-28T00:45:36Z

src/main/java/chess/repository/RoomJdbcDao.java

+        return jdbcTemplate.query("select * from room where id = ?", resultSet -> {
+            if (resultSet.next()) {
+                final int id = resultSet.getInt("id");
+                final String name = resultSet.getString("name");
+                final int findUserId = resultSet.getInt("user_id");
+                return new Room(id, name, findUserId);
+            }
+            return null;
+        }, roomId);


현재는 상관없지만
저는 RowMapper 같은 경우는 재사용되는 경우가 자주있어 전역변수로 뺴두는 편입니다!

public class RoomJdbcDao implements RoomDao { private final JdbcTemplate jdbcTemplate; private final RowMapper<Room> rowMapper = resultSet -> { ~~ };

다시 살펴보니 중복되는 부분이 있었군요..! 너무 좋은 피드백인 것 같습니다 👍
아래 남겨주신 피드백이랑 같이 적용해보도록 하겠습니다!

Gomding · 2023-03-28T00:53:00Z

src/main/java/chess/db/JdbcTemplate.java

+    public <T> T query(final String query, final RowMapper<T> rowMapper, final Object... parameters) {
+        final Connection connection = connectionPool.getConnection();
+        try (final PreparedStatement preparedStatement = connection.prepareStatement(query)) {
+            for (int i = 1; i <= parameters.length; i++) {
+                preparedStatement.setObject(i, parameters[i - 1]);
+            }
+            final ResultSet resultSet = preparedStatement.executeQuery();
+            return rowMapper.mapRow(resultSet);
+        } catch (SQLException e) {
+            throw new IllegalArgumentException(e.getMessage());
+        }
+    }


단일 결과를 리턴하는 query 메서드 와 다수(Collection) 결과를 리턴하는 query 메서드를 미리 생성해보면 어떨까요?
단일 결과, 다수 결과가 필요한 것은 자주 있는 일이라 미리 나눠놓으면 명확할 거라 생각해요 😄

단일 결과 리턴 -> queryForSingleResult
다건 결과 리턴 -> query
와 같이 메서드 이름도 나눠볼 수 있겠어요 🙇

아래처럼 사용할 수 있도록 해보는건 어떻게 생각하시나요? 😉

private final RowMapper<User> rowMapper = resultSet -> { final int id = resultSet.getInt("id"); final String findName = resultSet.getString("name"); return new User(id, findName); }; public User findByName(final String name) { return jdbcTemplate.queryForSingleResult("SELECT * FROM user WHERE name = ?", rowMapper, name); } public List<User> findAll(final String name) { return jdbcTemplate.query("SELECT * FROM user", rowMapper, name); }

해당 부분을 적용하고나서 코드가 너무 깔끔해진 것 같습니다 👍
감탄과 함께 리팩터링 진행했습니다 ㅋㅋㅋ 😄
사용하는 입장에서도 명확할 뿐더러 사용 방법도 더 편해진 것 같아요!

Gomding · 2023-03-28T00:55:42Z

src/test/java/chess/db/TestConnectionPool.java

+import java.sql.DriverManager;
+import java.sql.SQLException;
+
+public class TestConnectionPool implements ConnectionPool {


테스트용 DB 사용 👍

Gomding · 2023-03-28T01:03:25Z

src/test/java/chess/service/RoomServiceTest.java

+        mockRoomDao = new RoomDao() {
+            private final List<Room> rooms = new ArrayList<>();
+            private int index = 0;


해당 부분은 익명 클래스보다 RoomDaoStub 과 같은 클래스로 분리해봐도 되겠어요 😄

익명 클래스 선언때문에 중요한 테스트 코드가 아래로 가버린것같아요!

익명 클래스로 스터빙을 하다보니 중요한 테스트코드가 아래에 있어 테스트가 읽기 어려워졌네요!
말씀하신대로 따로 클래스로 분리하는 것이 더욱 깔끔해보입니다..
찰리의 코멘트는 왜 이리 다 값진가요.. 👍
남겨주신 코멘트 하나하나 적용해보면서 감동받지 않을 수 없었습니다.

greeng00se · 2023-03-28T06:11:55Z

꼼꼼한 리뷰 감사합니다 찰리! 🍫
제게 너무 필요한 부분을 코멘트 해주신 것 같았어요!
너무 재밌게 리뷰 반영한 것 같습니다!
저도 얼른 찰리처럼 시야가 넓어지고 싶습니다 👀

코멘트 남겨주신 부분 반영해서 다시 리뷰 요청합니다!

Gomding

안녕하세요 허브!
3,4 단계에서 많은것을 시도해주셨네요 😄
이번 미션은 이만 merge 하겠습니다 고생하셨어요!! 🙏

방학 잘 보내시고 레벨2도 화이팅입니다 👏 👏 👏 👏

과정 중에 궁금한 점 있으시면 언제든 DM 주세요~!!

Gomding · 2023-03-29T13:22:23Z

src/main/java/chess/db/JdbcTemplate.java

+            for (int i = 1; i <= parameters.length; i++) {
+                preparedStatement.setObject(i, parameters[i - 1]);
+            }
+            final ResultSet resultSet = preparedStatement.executeQuery();


이 부분도 중복 코드이니 추출해볼 수 있겠어요~

또한 ResultSet 도 자원해제가 필요한 객체입니다 😄

그렇네요! 👍 👍 다음과 같이 리팩터링 하겠습니다!

public <T> List<T> query(final String query, final RowMapper<T> rowMapper, final Object... parameters) { final Connection connection = connectionPool.getConnection(); try (final PreparedStatement preparedStatement = connection.prepareStatement(query); final ResultSet resultSet = executeQuery(preparedStatement, parameters)) { final List<T> result = new ArrayList<>(); while (resultSet.next()) { result.add(rowMapper.mapRow(resultSet)); } return result; } catch (SQLException e) { throw new IllegalArgumentException(e.getMessage()); } } private ResultSet �executeQuery( final PreparedStatement preparedStatement, final Object[] parameters) throws SQLException { for (int i = 1; i <= parameters.length; i++) { preparedStatement.setObject(i, parameters[i - 1]); } return preparedStatement.executeQuery(); } public <T> Optional<T> queryForSingleResult(final String query, final RowMapper<T> rowMapper, final Object... parameters) { final Connection connection = connectionPool.getConnection(); try (final PreparedStatement preparedStatement = connection.prepareStatement(query); final ResultSet resultSet = executeQuery(preparedStatement, parameters)) { if (resultSet.next()) { return Optional.of(rowMapper.mapRow(resultSet)); } return Optional.empty(); } catch (SQLException e) { throw new IllegalArgumentException(e.getMessage()); } }

Gomding · 2023-03-29T13:25:32Z

src/main/java/chess/view/input/InputView.java

+import java.util.List;
+import java.util.Scanner;
+
+public class InputView {


InputView 는 입력만 받도록 변경해주셨군요 👍

Gomding · 2023-03-29T13:31:14Z

src/main/java/chess/repository/UserJdbcDao.java

+    public void deleteAll() {
+        jdbcTemplate.executeUpdate("DELETE FROM user");
+    }


deleteAll 메서드는 테스트용 코드라고 생각돼요 😄
테스트 패키지 내에서만 관리하면 어떨까요?

사용하지 않는 deleteAll 메서드를 제거하고
테스트에서 기존에 생성해둔 jdbcTemplate 클래스를 이용하여 delete하는 방향으로 수정했습니다!

@BeforeEach void setUp() { jdbcTemplate.executeUpdate("DELETE FROM move"); }

greeng00se added 28 commits March 19, 2023 13:40

refactor: 스트림 미니 미션 테스트 Disable

e22fcb3

refactor: 보드 생성 책임을 담당하는 BoardGenerator 추가

8a78c7b

test: 체스판 검증 기능 가독성 개선

4e41311

refactor: static 제거

8659523

refactor: 사용하지 않는 메서드 제거

85e3615

refactor: EmptyPiece의 정적 팩터리 메서드 네이밍 변경

547058d

refactor: board 검증부 부정연산자 제거

d878f8b

refactor: 기물 메서드명 일관성있게 변경

b904300

refactor: 줄바꿈 일관성있게 변경

319563d

refactor: Board를 문자열로 변경해주는 Converter 클래스 추가

5d450b8

refactor: Position 패키지 분리

0441806

docs: 3단계 요구사항 정리

d9522c7

feat: 같은 생상인지 확인하는 메서드 추가

0119bd3

feat: 기물의 점수를 계산하는 기능 추가

de53b5d

refactor: 보드 상태패턴 적용

b1b6758

feat: 왕이 잡히면 게임이 종료되는 기능 추가

65739a2

feat: 결과를 반환하는 기능 추가

0ad4ac9

feat: 결과를 계산하는 클래스 추가

2e408fc

feat: 상태 출력하는 기능 추가

505143e

refactor: 커맨드 길이 검증 개선

392be91

docs: 요구사항 정리

a87dc9d

refactor: 상태 클래스명 변경

abceac6

refactor: Board의 Move가 Position을 받도록 수정

e986881

feat: 데이터베이스 연동기능 추가

7130470

feat: 체스게임 초기화 기능 추가

01fe9d9

refactor: ChessGame에서 MoveDto 사용하도록 수정

4e8d1c2

refactor: 템플릿 콜백 패턴 적용

87dc039

refactor: 매개변수명 수정 및 final 추가

2ebb089

Gomding requested changes Mar 22, 2023

View reviewed changes

refactor: Dao 예외 전환 적용, RowMapper 메서드 시그니처 수정

4746e46

greeng00se added 13 commits March 26, 2023 21:49

feat: ThreadLocal 값 제거 로직 추가

0ed5a87

refactor: 게임 진행 기능 수정

ecd5232

Merge remote-tracking branch 'origin/step2' into step2

e025164

# Conflicts: # src/main/java/chess/view/input/RoomInputView.java

refactor: nameDto 제거

1ffb77c

refactor: Action 하나만 사용하도록 수정

ef1ce69

refactor: static import 설정

1149eae

remove: 사용하지 않는 namedto 제거

682dd16

docs: 요구사항 정리

fb10477

refactor: 방 finished 필드 제거

9717a43

refactor: 방 미선택시 선택한 방 이름 none으로 보이도록 수정

515fbd1

remove: 사용하지 않는 메서드 제거

995fd19

docs: 다이어그램 추가

89d8395

fear: GameController에서 나가면 service에 올라간 board 제거

41e1643

greeng00se added 3 commits March 27, 2023 00:48

refactor: 메서드 위치 변경

172151a

remove: 사용하지 않는 생성자 제거

ba1baf4

remove: 사용하지 않는 변수 제거

8751e86

greeng00se commented Mar 26, 2023

View reviewed changes

Gomding requested changes Mar 28, 2023

View reviewed changes

greeng00se added 7 commits March 28, 2023 11:32

refactor: MainController Controller 인터페이스 구현하는 부분 제거

0d5f686

refactor: 인터페이스명 SubController로 변경

a8adb91

docs: 외래키 추가

1a13e1f

refactor: DAO optional 적용

e971899

refactor: rowMapper 재사용 가능하도록 필드로 설정

f9f953a

refactor: 각각의 DaoStub 이너 클래스로 생성

e24049a

refactor: InputView 중복 제거

602c24f

remove: 사용하지 않는 클래스 제거

9c8b418

Gomding approved these changes Mar 29, 2023

View reviewed changes

Gomding merged commit 40223ed into woowacourse:greeng00se Mar 29, 2023

	commandMapper.put(START, ignore -> start());
	commandMapper.put(START, commands -> start());
	commandMapper.put(MOVE, commands -> move(commands));

		System.out.println("> 회원가입 : register 이름 - 예) register charlie");
		System.out.println("> 로그인 : login 이름 - 예) login charlie");

[3, 4단계 - 체스] 허브(방대의) 미션 제출합니다. #529

[3, 4단계 - 체스] 허브(방대의) 미션 제출합니다. #529

Conversation

greeng00se commented Mar 21, 2023

Gomding left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

greeng00se Mar 22, 2023 • 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

greeng00se Mar 22, 2023 • 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

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

greeng00se commented Mar 26, 2023 • edited Loading

Choose a reason for hiding this comment

Gomding 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

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

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

Choose a reason for hiding this comment

greeng00se commented Mar 28, 2023

Gomding left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

greeng00se Mar 29, 2023 • 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

greeng00se Mar 22, 2023 •

edited

Loading

greeng00se Mar 22, 2023 •

edited

Loading

greeng00se commented Mar 26, 2023 •

edited

Loading

greeng00se Mar 29, 2023 •

edited

Loading