[디디] 체스게임 웹/DB 미션 제출합니다. #129

fucct · 2020-04-06T12:16:32Z

안녕하세요! 리뷰어님 DM으로 질문을 드렸는데 바쁘셔서 못보신 것 같아요!
미션 종료가 얼마 남지 않아서 일단 그동한 했던 2~3주차 미션도 함께 제출합니다! 함께 리뷰해주시면 감사하겠습니다.
혹시라도 DM을 못보셨을까봐 여기에 다시 질문 남기겠습니다!

안녕하세요 리뷰어님! 피드백 잘 받았습니다! 다름이 아니라 status와 나머지 구현에 대해 질문이 있는데요! 점수가 Running State에서도 계산되도록 하여서 실시간으로 사용자에게 자신과 상대방의 점수를 보여주려고 구현을 했었습니다. 마찬가지로 End State에서도 Board를 보고 결과를 알 수 있도록 하려고 구현을 했었습니다. 코치님들께도 구현부분에 있어선 자유롭게 해도 된다고 하셔서 이렇게 했고, 강의자료에도 정확하게 구현에 대한 예시가 없어서 헷갈려서 질문 드립니다. RunningState에서는 점수 계산 없이 보드만을 보여주고, End State에서는 Board를 보여주지 않고 점수만을 보여주어야 하는 건가요? 답변주시면 감사하겠습니다 😄

그리고 추가적인 질문도 드리겠습니다!
State 패턴을 사용했던 것이 도메인 상에선 관리가 좋고 배운것을 활용하기 위함이였는데요, 웹상에서 State 패턴이 오히려 구조를 복잡하게 하고 에러를 많이 발생시켜서 제거하고 End인지를 확인하는 메소드만 추가를 했는데 사실 어떤 설계가 더 좋은지 확실치 않네요! 조언주시면 반영하도록 하겠습니다.
두번째 질문은 web Controller가 Console Controller와 맞지 않아서 분리를 하였는데 이게 좋은 방법인지 잘 모르겠습니다. 실제로 컨트롤러를 여러개를 두고 사용을 하기도 하나요?
세번째 질문은 명령어에 따라 세부적으로 받아야 하는 response의 내용도 다양하게 달라지는데, 그런 경우에 Dto 클래스를 분리하는 방식으로 설계를 하는게 좋을까요? 아니면 모든 데이터를 가질 수 있는 Dto를 만들어서 사용하지 않는 부분을 null로 초기화 해주는 방식이 좋을까요?

답변주시면 감사하겠습니다!😅

- Position은 내부적으로 file과 rank를 알고 있다. - Position을 map으로 캐싱해서 String 입력 값에 따라 해당하는 position 객체를 반환

- Piece는 자신의 위치를 안다 - Piece는 자신이 속한 Player를 안다

- Board를 초기화하기 위한 BoardInitializer 인터페이스 추가 - Board initializer가 사용할 초기 데이터인 InitialPieceRepository 추가 - EnumRepositoryBoardInitializer에서 repository를 참고하여 board 초기화

Todo. - 장애물이 있을 경우 판단하는 로직 구현 필요 - Knight를 제외한 다른 piece의 이동 규칙 구현 필요

- 게임의 진행 상태를 나타내는 State 생성 - Application 로직에 있던 내용을 Controller로 이동 - InputView의 리팩토링 - Board 출력을 위한 ResponseDto 생성 - OutputView에서 ResponseDto로 출력하는 로직 구현 - Piece의 색에 따라 toString 구현 - 한 게임을 관리하는 ChessGame 생성 - 이동을 위한 파라미터 객체인 MoveParameter 생성

- EnumRepository에서 생성된 피스를 가지지 않고, 피스 generator를 가지고 있도록 변경 - EnumRespositoryInitializer에서는 해당하는 generator로 피스를 생성하여 보드 초기화

- 이동 규칙에 적합하게 움직이는 지 테스트 - 중복되는 메서드를 템플릿 메서드로 분리 - PieceState를 적절하게 반환하도록 구현

- Piece의 상태에 따라 Ready, Running, End로 분리

- Rook Test 구현 - Direction 개념 구현 - Direction 에 맞게 position 이동 구현 - Rook 이동 구현

- Bishop Test 구현 - Direction 이 음수도 나오도록 수정 - Direction 에 맞게 position 이동 수정 - Bishop 이동 구현

- King Test 구현 - Direction 에 존재하지 않는 target의 경우 예외처리 - King 이동 구현

- 현재 Trun인 플레이어만 move가 가능하도록 구현 - 각 Piece 이동에 맞게 테스트 refactor - Pawn 이동 규칙에 맞게 분리

- 플레이어 체크 안되는 버그 수정

- 접근제어자 추가 - 불필요한 객체 삭제 - 매직넘버 상수화 - 가독성이 떨어지는 메서드 분리

- indent 줄이기 위해 메서드 분리 - status 결과 객체 구현 - 결과 계산시 종료 구현

- 컨벤션 리팩토링 - 테스트 에러 수정

- state 패턴 삭제 - service layer 추가 - indent 규칙에 맞게 리팩토링 - 기타 컨벤션에 맞게 수정

- Index page에 접근가능하도록 구현 - Index page에서 현재 존재하는 방 목록 보여주도록 구현 - DB에 접근하여 방 목록을 가져오도록 구현

- 저장시 DB에 저장 - 불러오기 시 DB 로부터 게임 load - 진행중인 게임을 저장하지 않고 종료시 DB 삭제 - 진행중인 게임에서 새 게임 플레이 가능

jeonghoon1107

안녕하세요 디디!
몇가지 피드백을 남겼으니 확인부탁드려요 :)

제가 이야기 한 것은 State에서 점수를 계산하는 것을 이야기 한 것이에요!
상태 관리 -> 주로 이벤트 기반으로 상태를 관리하는데 현재 구조에서는 한계가 있는 것 같아 보여요. 이 부분에 대해서 더 궁금하시면 DM 주세요!
현재는 controller를 분리해도 괜찮은 것 같아요. 조금 더 나아가고 싶다면 레이어를 하나 더 두는 것이 좋은 방법 같아요. adapter라던가..
null로 초기화하는 것은 피하는 것이 좋을 것 같아요.

jeonghoon1107 · 2020-04-10T12:35:35Z

src/main/java/chess/dao/JDBCChessDAO.java

+import java.util.*;
+import java.util.stream.Collectors;
+
+public class JDBCChessDAO implements ChessDAO {


카멜케이스를 지켜주세요 :)

jeonghoon1107 · 2020-04-10T12:36:34Z

src/main/java/chess/dao/JDBCChessDAO.java

+            Long id = getCurrentGameId(con);
+            addBoard(id, chessGame);
+            return id;
+        } catch (SQLException e) {


전체적으로 exception을 잡아서 바로 다시 던지는 행위는 불필요해보이네요.

jeonghoon1107 · 2020-04-10T12:36:54Z

src/main/java/chess/dao/ChessDAO.java

+import java.util.List;
+import java.util.Map;
+
+public interface ChessDAO {


카멜케이스!

jeonghoon1107 · 2020-04-10T12:38:48Z

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

+            return new ResponseDto(chessGame.getBoardAndString(), chessGame.getTurn(),
+                    chessGame.getStatus(), id);
+        } catch (SQLException | IllegalArgumentException | UnsupportedOperationException e) {
+            return new ResponseDto(e.getMessage());


메시지를 던져주는 것 너무 좋네요 👍

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

jeonghoon1107 · 2020-04-10T12:42:17Z

src/main/java/chess/dao/JDBCChessDAO.java

+public class JDBCChessDAO implements ChessDAO {
+    public Connection getConnection() {
+        Connection con = null;
+        String server = "localhost:13306"; // MySQL 서버 주소


왜 상수를 사용하는 것이 유리한지에 대해 이해를 하셨나요?

안녕하세요 리뷰어님! 혹시 Connection도 상수로 사용해도 문제가 없나요? 혹시 문제가 될까봐 상수로 사용하지 않았는데 상수로 바꾸는 것이 좋을까요?

jeonghoon1107 · 2020-04-10T12:48:41Z

src/main/java/chess/dao/JDBCChessDAO.java

+        try (Connection connection = getConnection()) {
+            Map<Position, PieceState> board = getCurrentBoard(id, connection);
+            Turn turn = getCurrentTurn(id, connection);
+            if (turn == null) return null;


https://google.github.io/styleguide/javaguide.html#s4.1-braces

jeonghoon1107 · 2020-04-11T05:41:57Z

src/main/java/chess/controller/ChessWebController.java

+    }
+//
+//
+//        get("/", (req, res) -> {


불필요한 코드들인가요?

jeonghoon1107 · 2020-04-11T05:54:22Z

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

+        return commands.get(requestDto.getCommand()).apply(requestDto);
+    }
+
+    public ResponseDto start(RequestDto requestDto) {


requestDto 사용하는 변수인가요?

jeonghoon1107 · 2020-04-11T06:11:07Z

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

+        long id = requestDto.getId();
+        ChessGame chessGame = chessGames.get(id);
+        List<String> parameter = requestDto.getParameter();
+        ResponseDto responseDto = new ResponseDto("");


해당 위치에서 객체를 만들어 주고 set을 할 필요는 없을 것 같아요!

jeonghoon1107 · 2020-04-11T06:12:14Z

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

+    }
+
+
+    private ResponseDto unknown(RequestDto requestDto) {


ResponseDto안에 선언해놓고 사용할 수 있지 않을까요?

jeonghoon1107 · 2020-04-11T06:14:49Z

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

+
+    @BeforeEach
+    void setUp() throws SQLException {
+        chessService.start(null);


테스트끼리 영향을 받을 수 있기 때문에 저장소에 있는 데이터는 각 테스트 케이스마다 deleteAll을 실행하는 것이 좋을 것 같아요.

fucct · 2020-04-12T06:38:17Z

답변 감사합니다! 제가 이번에 request 와 response 를 처음 사용해보려고 했는데 원하는 대로 깔끔하게 구현하지 못했네요 ㅠㅠ requestDto에 요청 값을 저장할 때, 사용자가 입력한 그대로인 String 을 저장해야 하는지, 아니면 사용자의 입력을 적절히 변환시켜서 저장해야 하는지 잘 모르겠습니다.. 그리고 말씀하신 것 처럼 response에 null로 초기화 되는 필드들이 많은데 이 필드들을 분리해서 여러개의 dto로 관리해야 하나요? 아니면 다른 방식으로 해야할까요? 조언해주시면 감사하겠습니다!😅

jeonghoon1107

안녕하세요 디디!
미션 진행하느라 고생하셨습니다 :)
추가적으로 궁금한 부분은 언제든 DM 주세요!

jeonghoon1107 · 2020-04-17T13:03:08Z

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

+
+    public ChessService(ChessDao chessDAO) {
+        this.chessDAO = chessDAO;
+        commands.put(Command.START, null);


빼거나 기본 request를 넣어주면 어떨까요?

jeonghoon1107 · 2020-04-17T13:06:18Z

src/main/java/chess/controller/dto/ResponseDto.java

+import java.util.Objects;
+
+public class ResponseDto {
+    private static final ResponseDto unknownResponse = new ResponseDto("알 수 없는 명령어 입니다.");


접근제어자를 public으로 변경하고 사용해도 좋을 것 같아요!

fucct added 28 commits March 24, 2020 15:07

docs : README.md 업데이트

e041e93

feat : Coordinate 구현

ce4c145

[feat] Position 객체 생성

025f397

- Position은 내부적으로 file과 rank를 알고 있다. - Position을 map으로 캐싱해서 String 입력 값에 따라 해당하는 position 객체를 반환

feat: Piece, Knight 일부 구현

f9834ca

- Piece는 자신의 위치를 안다 - Piece는 자신이 속한 Player를 안다

feat: board 생성

10389c0

- Board를 초기화하기 위한 BoardInitializer 인터페이스 추가 - Board initializer가 사용할 초기 데이터인 InitialPieceRepository 추가 - EnumRepositoryBoardInitializer에서 repository를 참고하여 board 초기화

feat: board에서 move에 대한 로직을 간단하게 구현

22d43cf

Todo. - 장애물이 있을 경우 판단하는 로직 구현 필요 - Knight를 제외한 다른 piece의 이동 규칙 구현 필요

refactoring: EnumRepository의 구현을 변경

12b034a

- EnumRepository에서 생성된 피스를 가지지 않고, 피스 generator를 가지고 있도록 변경 - EnumRespositoryInitializer에서는 해당하는 generator로 피스를 생성하여 보드 초기화

rafactoring/feat : Knight, MovedPawn 테스트 및 이동 구현

30d8e44

- 이동 규칙에 적합하게 움직이는 지 테스트 - 중복되는 메서드를 템플릿 메서드로 분리 - PieceState를 적절하게 반환하도록 구현

refactoring/feat : PieceState 구현

2fc69f9

- Piece의 상태에 따라 Ready, Running, End로 분리

feat : Rook 이동 구현

0573e77

- Rook Test 구현 - Direction 개념 구현 - Direction 에 맞게 position 이동 구현 - Rook 이동 구현

refactoring/feat : 기존 이동 로직 수정 및 Bishop 이동 구현

12f4db6

- Bishop Test 구현 - Direction 이 음수도 나오도록 수정 - Direction 에 맞게 position 이동 수정 - Bishop 이동 구현

refactoring/feat : King 이동 구현

5c0fbe2

- King Test 구현 - Direction 에 존재하지 않는 target의 경우 예외처리 - King 이동 구현

refactoring/feat : 전체 Piece 이동 로직 리팩토링, Turn 구현

208e7ae

- 현재 Trun인 플레이어만 move가 가능하도록 구현 - 각 Piece 이동에 맞게 테스트 refactor - Pawn 이동 규칙에 맞게 분리

feat: Status 로직 구현, King이 죽으면 게임종료 구현

6595321

refactor : king 죽음 리팩토링

f7234e7

- 플레이어 체크 안되는 버그 수정

refactor : 컨벤션 리팩토링

d97af7b

- 접근제어자 추가 - 불필요한 객체 삭제 - 매직넘버 상수화 - 가독성이 떨어지는 메서드 분리

refactor/feat : 설계 리팩토링

3eb19bf

- indent 줄이기 위해 메서드 분리 - status 결과 객체 구현 - 결과 계산시 종료 구현

refactor : 설계 리팩토링

3e7ee5a

- 컨벤션 리팩토링 - 테스트 에러 수정

refactor : 리뷰 반영

e89e26d

refactor : 불필요한 설계 수정

8e14f90

- state 패턴 삭제 - service layer 추가 - indent 규칙에 맞게 리팩토링 - 기타 컨벤션에 맞게 수정

refactor : 불필요한 테스트 삭제

0ba02cc

feat : Status Test 추가

3f688e4

refactor : package 이름 수정

f62a683

임시 commit

ed32f9a

feat : Web 환경 설계 및 DB 연동

b2ea5e9

- Index page에 접근가능하도록 구현 - Index page에서 현재 존재하는 방 목록 보여주도록 구현 - DB에 접근하여 방 목록을 가져오도록 구현

feat : 게임방 기능 구현

2754a72

- 저장시 DB에 저장 - 불러오기 시 DB 로부터 게임 load - 진행중인 게임을 저장하지 않고 종료시 DB 삭제 - 진행중인 게임에서 새 게임 플레이 가능

refactor : 불필요한 메서드 삭제

2f4375b

fucct force-pushed the pair-db branch 2 times, most recently from 089314f to 2f4375b Compare April 8, 2020 11:51

feat : ChessService 테스트 구현

579fa19

jeonghoon1107 suggested changes Apr 11, 2020

View reviewed changes

jeonghoon1107 reviewed Apr 11, 2020

View reviewed changes

fucct added 9 commits April 11, 2020 16:10

fix, style : 완료된 게임 저장시 버그 수정, 클래스 명 수정

c3dcc12

refactor : 불필요한 throw 문 삭제

1014a66

refactor : 불필요한 setter 메서드를 생성자로 대체

7a9b720

style : local 변수를 상수로 대체

e1ef47e

style : 일부 stream 표현을 foreach 로 대체

640711a

fix : 우승 표시가 올바르지 않게 표시되던 버그 수정

a9474f0

style : 불필요한 문장 삭제, 변수명 수정

b1da173

refactor : 테스트 저장소 초기화

0310a70

refactor : responseDto에 unknown 필드 추가

efbcbff

fucct closed this Apr 12, 2020

fucct reopened this Apr 12, 2020

jeonghoon1107 approved these changes Apr 17, 2020

View reviewed changes

jeonghoon1107 merged commit 250cb50 into woowacourse:fucct Apr 17, 2020

[디디] 체스게임 웹/DB 미션 제출합니다. #129

[디디] 체스게임 웹/DB 미션 제출합니다. #129

Uh oh!

Conversation

fucct commented Apr 6, 2020

Uh oh!

jeonghoon1107 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fucct commented Apr 12, 2020

Uh oh!

jeonghoon1107 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jeonghoon1107 left a comment •

edited

Loading