[4, 5단계 - 체스] 삭정(성정석) 미션 제출합니다.#225
Conversation
- ChessBoard를 List로 변경 - Piece를 상속하는 체스 말 객체 생성
- 이동 제약 사항 구현
- 각 기물 규칙에 따라 이동 - Pawn, Rook 움직임 테스트
- 동일하게 처리하는 조건문 메소드로 분리
- Direction에서 직진, 대각선 분리 - 직진, 대각선 이동 따로 처리
- row, column 추가로 인한 변경
- 갈 수 있는 위치 판단 로직 변경
- ChessBoard에서 점수 계산 - 점수 산정을 위해 Type에 score 추가
- Result로 점수와 승패 판별
- 움직일 수 있는 위치 찾는 로직을 Path로 이동 - Piece의 Position 제거, Square 에서 이동 담당
- 테스트 추가 - Position 사용하는 코드 수정
- ChessBoard가 빈 Square를 가지고 Piece 추가하도록 수정
room_id, turn, state 칼럼을 가지도록 변경
JunHoPark93
left a comment
There was a problem hiding this comment.
안녕하세요 삭정! 제이입니다.
미션 구현하시느라 고생많으셨어요 많이 힘들었죠?
질문주신부분들은 코멘트로 남겨두었어요! 확인해보시고 또 궁금한점 남겨주세요 🙏
| } | ||
|
|
||
| private void validateRoomExistence(Room room) { | ||
| if (room == null) { |
There was a problem hiding this comment.
room을 반환하는 쪽에서 Optional을 사용하면 이렇게 명시적으로 검사하는 부분들을 없애볼 수 있을거에요!
| List<String> moveCommand = getMoveCommand(req); | ||
| Map<String, Object> model = webUIChessController.movePiece(moveCommand); | ||
| if (model == null) { | ||
| res.status(400); |
There was a problem hiding this comment.
모델이 null이라면 400을 내려주고있어요.
ChessController에서 status를 내려주고 그 status값으로 분기를 해보면 어떨까요?
| return getRoomToRender(chessGame); | ||
| } catch (Exception e) { | ||
| System.out.println(e.getMessage()); | ||
| return null; |
There was a problem hiding this comment.
위에 말씀드린 부분입니다. 문제가 생기거나 움직일수없을 때 null을 내려주는걸로 보이는데 상태를 같이 넘겨주면 null을 반환하는 부분이 없어질거에요!
그 객체는 내부적으로 HttpStatus 와 Map<String,Object>를 가질수있을거같네요
| import java.util.List; | ||
|
|
||
| public class RoomDAO { | ||
| private final Gson gson = new Gson(); |
There was a problem hiding this comment.
Gson을 여기서도 생성해주고 컨트롤러에서도 필드로 생성해주고있어요. 커스텀하여 사용하게 된다면 양쪽다 수정을 해야할건데 한군데에서 관리해볼수 있을까요~?
| public class RoomDAO { | ||
| private final Gson gson = new Gson(); | ||
|
|
||
| public Connection getConnection() { |
There was a problem hiding this comment.
RoomDAO내부에서 Connection을 관리하고 있어요. 다른 DAO가 생겼었을때는 해당 DAO에서 같은 작업을 반복해주어야할까요?
커넥션이란것을 따로 관리하는 방법은 없을까요?
|
|
||
| public class WebUIChessController { | ||
| private final Gson gson = new Gson(); | ||
| private final RoomDAO roomDAO = new RoomDAO(); |
There was a problem hiding this comment.
질문주신 부분같아요!
로직이 크게 많지않아 컨트롤러에서 간단한 로직들을 수행하고 반환하고있는것으로 보여요. 그랬을시에 컨트롤러에서 바로 데이터계층에 접근하는것도 크게 나쁘지는 않아보입니다. MVC라는것을 적용해보는데있어서는 잘 나누어주셨고요.
다만 DAO 내부에 대해서 코멘트를 남겨두었어요.
그리고 웹에서 C (controller)의 역할이 어떤것일지에 대해서 조금만 더 찾아보시면 좋을거같아요. (삭정이 만들어주신 현재 구조에서 C가 담당하는 역할을 줄이는 쪽으로 가면 더 좋을거같아요. 지금 반영하라는건 아니고 고민만 해보셔요 🙏)
찾아보시게되면 레이어드아키텍쳐라는 키워드를 마주하실수있을거같은데요, 찾아보다보면 궁금증이 풀리지않을까합니다.
추후 미션을 진행하시면서 점차 알게되실거라 여기까지만 남길게요!
| import chess.domain.game.Result; | ||
| import chess.domain.piece.Piece; | ||
| import chess.domain.piece.feature.Color; | ||
| import chess.domain.feature.Color; |
There was a problem hiding this comment.
두번째 질문주신 부분인거같아요!
(dto 관련해서 질문을 주셔서 Output관련된 클래스를 찾다가 여기만찝혀서 여기에 남겨요 😂)
요구사항에 도메인을 최대한 건드리지 마라고 해서 일단 DTO를 만들지 않고 기능을 구현해보았습니다.
라고 해주셨는데 그렇다고해서 도메인자체로 통신을 하라고 말씀을 하시진 않았을거에요. OutputView를 보면 뷰관련된 클래스에 도메인자체가 넘어가는걸 볼 수 있는데 그렇게 된다면 뷰에서 도메인을 조작할 수 있는 가능성이 있어요. 그렇기 때문에 꼭 필요한 값들만 dto로 만들어서 넘기는게 좋다고 생각해요. 해당작업은 컨트롤러에서 해주면 될거구요! 고민만 해보시고 코멘트 남겨주세요!
혹시 답변이 되었을까요?
주요 리팩토링 사항1. Response객체 생성컨트롤러가 Map<String, Object>형태의 model을 넘겨주는 방식에서 Response 객체를 넘겨주는 방식으로 바꿔 보았습니다. 제이의 피드백이 의도한 바가 이게 맞나요? ㅎㅎ 확인 부탁드립니다. 2. DTO 생성DTO를 한 번 만들어 봤습니다. 현재 Response 객체 내부에서 렌더링을 위한 model을 만들어 줄 때 필요한 DTO들을 생성해서 넣어주는 방식으로 구현해 봤습니다. 사실 이렇게 하는게 맞는지는 좀 긴가민가 합니다 ㅋㅋ. 3. db connectiondb connection을 전담하는 유틸성 객체 databseTransaction을 만들어서 db와의 연결을 담당하게 만들었습니다. 혹시 더 좋은 방법이나 현업에서의 관행(?) 같은게 있다면 알려주시면 감사하겠습니다.
이 부분에 대해서는 좀 더 많은 공부가 필요할 것 같아 이번 미션에서는 적용하지 않으려고 합니다. 제이의 힌트를 바탕으로 제대로 공부해서 레벨 2부터 적용 해보려구요 ㅋㅋ. 이해하기 쉽게 잘 설명해주셔서 정말 감사합니다. |
JunHoPark93
left a comment
There was a problem hiding this comment.
리뷰 반영하시느라 고생많으셨습니다 🙇♂️
개선해주신 부분들에 대해서 몇가지 코멘트 더 남겨두었어요.
시간되실 때 확인부탁드려요 🙏
방학 잘 보내시고 다음 레벨 때 뵐게요
| import java.util.Map; | ||
|
|
||
| public class Response { | ||
| private static final int HTTP_STATUS_SUCCESSFUL = 200; |
There was a problem hiding this comment.
클래스 구조가 디폴트면 해당 상수를 이용하게 되어있고 그렇지 않다면 전달해준 httpStatus 코드를 사용하고있습니다.
지금 미션에서 상태코드의 값이 중요하진 않지만, 클래스 역할이 모호하여 코멘트남겨요!
Response는 응답만을 담당하게 될 클래스로 보이는데, 상태코드는 온전한 값으로 넘기는게 맞지 않을까요?
제대로 되지 않은 코드값을 넘겨주어도 Response 객체가 만들어지게되어 응답이 내려갈것으로 보여요.
상태코드를 따로 두어 (enum 등으로) 넘겨주거나 그렇지 않다면 Response.ok(...) 혹은 Response.notFound(...) 등과같은 생성자를 두고 내부에서 상태코드값을 세팅하는 방안으로 하는게 조금은 더 나아보이는데 어떻게 생각하시나요?
(다시 말씀드리지만 http 상태코드에 대해서 말씀드리는것이 아닌 클래스 구조에 관해서 입니다!)
There was a problem hiding this comment.
제이의 의견에 동의합니다. 너무 사용하는데 있어서 편의만 생각한 것 같네요. 머지 해주셨지만 방학동안 한 번 고쳐보겠습니다 ㅎㅎ.
| return response; | ||
| } catch (Exception e) { | ||
| System.out.println(e.getMessage()); | ||
| return new Response(500); |
| ResultSet rs = pstmt.executeQuery(); | ||
|
|
||
| Room room = Optional.ofNullable(getRoom(rs)) | ||
| .orElseThrow(IllegalArgumentException::new); |
There was a problem hiding this comment.
Optional 타입으로 반환해도 되긴하지만 어쨌든 null을 반환하는 부분이 사라져서 좋네요 👍
| import java.util.List; | ||
|
|
||
| public class WebUIChessController { | ||
| public static final Gson gson = new Gson(); |
There was a problem hiding this comment.
Gson을 관리하는 쪽이 한쪽이 되었지만, 퍼블릭 상수로 관리가 되는것보다는 컨버팅을 담당하는 클래스를 두고 내부에 Gson을 숨겨보는것도 좋을거같아요~
There was a problem hiding this comment.
Gson을 내부적으로 활용하는 유틸성 객체를 만들라는 의도이시죠? ㅎㅎ 이 부분도 반영해보겠습니다!
|
Optional내용
링크Web Application (Spark Java)내용
링크DTO내용
|
안녕하세요 제이!
이번 미션을 통해 웹을 처음 다뤄 보았습니다. 재밌기도 했지만 갑자기 너무 방대한 양의 지식을 습득해야 해서 힘들었네요 ㅠ..ㅠ 그래서 우선 목표로한 기능을 구현하는 것 에 최대한 집중해서 완성시켜 보고자 했습니다.
궁금한 점
1. mvc 설계
앞에서 말했듯이 웹에 대한 사전지식이 하나도 없어서 어떻게 구조(설계)를 가져가야할 지 막막했습니다. 콘솔로만 구현할 때는 mvc가 어느정도 명확했습니다. 하지만 web쪽으로 넘어오니 mvc를 어떻게 적용해야하는지 도저히 감이 오지 않았습니다.
현재 제 코드의 설계 부분에 있어서 제이의 조언을 얻고 싶습니다. 그리고 혹시 참고해서 공부할 만한 좋은 자료나 책이 있다면 알려주시면 감사하겠습니다.
2. DTO, DAO
다른 크루들을 보니 DTO를 적용해서 레이어 간 데이터를 주고 받는 것을 알 수 있었습니다. 요구사항에 도메인을 최대한 건드리지 마라고 해서 일단 DTO를 만들지 않고 기능을 구현해보았습니다. DTO 만약에 만든다면 어떤식으로 만들어야하는지 제이의 조언을 얻고 싶습니다.
DAO도 이번 우테코 씨유 강의를 통해 알게 되었습니다. 현재 제 코드에서는 컨트롤러에서 필드로 DAO를 가지고 있습니다. 그리고 이를 db와의 트랜잭션이 필요할 때 마다 사용하고 있습니다. 이렇게 사용하는 것이 맞는 방법인지 궁금합니다.