[4, 5단계 - 체스] 기론(김규철) 미션 제출합니다. #357
Conversation
- .sql 작성 - html, css, js 작성 - BoardDao, PieceDao 생성
board.create()구조 변경에 따른 test코드 수정
- end -> restart
- refactor: 디렉토리 구조 정리
|
|
||
| import static spark.Spark.get; | ||
| import static spark.Spark.port; | ||
| import static spark.Spark.staticFileLocation; | ||
|
|
||
| public class WebApplication { |
There was a problem hiding this comment.
웹에서 한 게임을 완료한 후 정상적으로 게임을 재시작할 수 없네요 😂
관련 재현 영상은 용량이 큰 관계로 DM 으로 보내드릴게요!
There was a problem hiding this comment.
board를 초기화 하고 piece들도 초기화 해야하는데 기존의 piece들의 position을 수정하지 않고 새로운 초기화된 piece 64개를 만들어서 오류가 생겼었네요!
initBoard()로직을 기존의 piece들의 position을 초기화하는 방식으로 수정했습니다!
| import chess.board.piece.Empty; | ||
| import chess.board.piece.Piece; | ||
| import chess.board.piece.Pieces; | ||
| import chess.board.piece.position.Position; | ||
|
|
||
| import java.util.List; | ||
|
|
||
| public final class Board { |
There was a problem hiding this comment.
콘솔에서 게임을 플레이할 때에 의도와 다른 예외 메시지가 발생해요.
white가 b2에서 b4로 이동한 후에 다음 자신의 턴에 다시 b2에서 b3로 이동하려할 때에 white의 차례임에도 불구하고 현재 차례가 아닙니다. 라는 메시지가 보여지네요!
도메인에 수정이 가해지며 콘솔 체스에도 영향이 간 모양이에요! 확인해볼까요~?
There was a problem hiding this comment.
Team에서 none인 팀까지 자신의 팀이라고 생각되어서 적절한 메시지가 안 갔었네요!
team비교하는 로직을 수정해서 고쳤습니다!
There was a problem hiding this comment.
확인했어요~!
리팩터링을 진행하며 기존 로직에 변화가 발생하여 이런 문제가 생겼죠?!
이런 상황을 방지하려면 테스트 코드를 꼼꼼하게 작성하면 좋을거에요.
|
|
||
| public void run() { | ||
|
|
||
| Gson gson = new Gson(); |
There was a problem hiding this comment.
gson 인스턴스를 static으로 선언하여 여러 ChessWebController 인스턴스에서 재사용해도 괜찮아보여요!
There was a problem hiding this comment.
아하 매번 api통신할 때마다 gson인스턴스를 생성하지 않고 미리 정적으로 만들어놓고 사용하는 방식이군요!
좋습니다!😃
| private final double blackTeam; | ||
| private final double whiteTeam; |
There was a problem hiding this comment.
검정 팀의 점수, 하얀 팀의 점수의 의미입니다!
Dto 이름이 ScoreDto라서 그냥 팀 이름만 적었는데 가독성이 떨어지는 것 같네요.
blackTeamScore,, whiteTeamScore로 수정했습니다!
| }); | ||
| } | ||
|
|
||
| private BoardDto convertToDto(Board board) { |
There was a problem hiding this comment.
기론의 첫 질문에 대해서 이야기해보죠!
DTO에 값을 전달하는 역할만을 부여하신 이유는 무얼까요? 즉 DTO는 왜 사용하는걸까요?
이전 단계 MR에서 도메인과의 연결을 끊기 위해 DTO를 사용하셨어요. 이로 유추해보면 변동성이 높은 뷰와의 연결을 없애기 위해 DTO를 사용하는 곳에서 도메인이 아닌 원시 값을 이용하도록하는 게 목적이겠네요!
그렇다면 이를 생성할 때는 도메인을 알고 있어도 게터로 원시값만 노출한다면 컨트롤러에 부가적인 메서드를 만들지 않고 충분히 목적을 달성할 수 있을까요~?
There was a problem hiding this comment.
아 그렇네요! dto가 도메인을 의존하는 것은 어떻게 보면 당연하네요(?) 도메인의 정보를 넘겨주려고 할 때, 사용되기 때문에 도메인이 혹시나 변경되면 dto는 당연히 변경되야하니깐요!
오히려 도메인에서 dto변환 로직을 갖는 게 문제가 되겠군요! 많은 변동 사항이 있는 dto로직을 도메인 안으로 끌고 오면, 도메인 내부의 순수한 비즈니스 로직만을 파악하기 어렵다고 생각되네요! 또한 dto로 변환할 때, dto에 도메인을 맞추는(?)상황이 생기는 것을 예방할 수 있을 것 같습니다!
There was a problem hiding this comment.
맞아요 컨트롤러를 도메인이라고 부르지는 않지만 컨트롤러도 너무 복잡해질 수 있기에 이 책임을 DTO에게 양도한다고 생각하시면 될거 같아요~! 현업에서도 이런 패턴으로 많이 구현한답니다!
src/main/resources/static/src/App.js
Outdated
| if (isChoiced === true) { | ||
| toTarget = event.target | ||
| isChoiced = false; | ||
| } | ||
| if (toTarget.id != "") { |
There was a problem hiding this comment.
JS에서 값을 비교할 때에 ===와 == 의 차이에 대해서 알고 계신가요~?
There was a problem hiding this comment.
간단하게
== 은 껍데기 비교라고 생각해서 254 == '254' 면 true,
=== 은 엄격한 비교로서 본질이 같은지 까지 비교한다고 생각했습니다.
그래서 254 == '254' 면 false라고 생각했었어요!
일반적으로는 ==을 주로 사용하고
true와 같은 boolean타입은 true나 false와 같이 이분적인 답만 나오므로 좀 더 엄격히 비교하도록 ===을 사용했습니다!
There was a problem hiding this comment.
오호 알고 계신 내용을 적어주셔서 감사해요!
==는 형변환을 하여 값을 비교하고 ===는 형변환을 하지 않아요.
JS는 타입이 없는 언어라 형변환에 대해서 깐깐하게 생각해주시면 좋아요.
그렇기에 기본적으로 ===를 사용하고 필요한 경우 개발자가 직접 형변환을 해주는 방식이 좋답니다~!
There was a problem hiding this comment.
아 그렇군요! 완전 반대로 사용하고 있었네요..ㅎㅎ 기본적으로 ===을 사용하도록 수정했습니다!
| } | ||
|
|
||
| @Test | ||
| void updatePieceByPosition() { |
There was a problem hiding this comment.
요 부분은 해당 라인에 공백을 넣기보다는 공백을 통해 테스트 메서드의 본문을 의미있는 단위로 나눠보면 좋겠다는 의미였어요.
@Test
@DisplayName("king과 black으로 타입과 팀을 바꾸면 db에 저장되고 다시 불러왔을 떄, 타입과 팀이 king과 black으로 나온다.")
void updatePieceByPosition() {
Piece piece = pieces.getPieces().get(0);
String newType = "king";
String newTeam = "black";
pieceDao.updatePieceByPositionAndBoardId(newType, newTeam, piece.getPosition().name(), boardId);
List<Piece> updatedPieces = pieceDao.findAllByBoardId(boardId);
Piece updatedPiece = updatedPieces.get(0);
assertThat(updatedPiece.getType()).isEqualTo(newType);
assertThat(updatedPiece.getTeam().value()).isEqualTo(newTeam);
}이런 느낌이랍니다~
제가 더 상세한 피드백을 드렸어야 했네요 😊
There was a problem hiding this comment.
앗 죄송합니다.. 좀 더 자세히 봤어야 했는데 너무 안일하게 넘어갔었네요 😂
| } | ||
|
|
||
| @Test | ||
| @DisplayName("piece들이 없는 board를 만든 후, initBoard호출하면 piece들까지 초기화된다.") |
There was a problem hiding this comment.
아 기존에 모킹 사용하기 전에 코드에서 적용되었던 내용이었네요.
mock 객체를 생성할 때, 만들어진 board의 id를 fakePiece들에게 인자 값으로 넣어둬서 바로 초기화된 것 같습니다!
| import java.util.Map; | ||
| import java.util.Optional; | ||
|
|
||
| public class MockBoardDao implements BoardDao { |
There was a problem hiding this comment.
모킹을 적용하신 점 훌륭합니다!
기론은 모킹을 적용할 때 어떤 장점이 있다고 생각하시나요?
There was a problem hiding this comment.
적용하면서 여러 장점을 얻었습니다.
제가 느꼈던 장점으로는
-
service테스트에만 집중할 수 있다.
- 실제 db에 저장되지 않으므로 db를 관리하지 않고 테스트를 할 수 있었어요! 그로 인해 service로직에만 집중할 수 있었습니다. (기존에는 테스트하다가 db가 지저분해지면 지워주고를 반복했어요ㅜㅜ)
- db의존성 제거로 명확한 단위테스트!
-
mock을 사용하지 않았을 때는 테스트하고 싶은 데이터를 직접 db에 넣고 수행해야했는데, mock을 사용하면
put()매서드로 간단하게 테스트하고 싶은 데이터를 넣을 수 있었어요!- 테스트하기 유연해졌다!
-
dao테스트와의 분리(?)
- 위에 1번과 비슷할 수 있는데 service테스트를 하면서 dao를 통해 db의 데이터를 불러오니 테스트가 에러가 났을 때, 이게 dao에서 불러오다가 잘못된건지 service로직에서의 에러인지를 구분하는데 어려웠습니다.
There was a problem hiding this comment.
맞아요. 특정한 부분에 대한 테스트를 하고 싶을 경우에 다른 의존성을 끊어줄 수 있죠!!!
모킹에 대해서는 여러가지 의견이 많은데 지금은 기론이 느끼신 장점으로 충분합니다!
나중에 이 책도 읽어보시면 테스트에 대한 개념을 잡는 데에 도움이 될거에요. 지금은 조금 어려울 수 있어서 우테코 끝날 즈음에 읽기를 추천드려요!
http://www.yes24.com/Product/Goods/104084175
There was a problem hiding this comment.
책 추천 감사합니다!
레벨 1동안 테스트를 꾸준히 작성했는데도 아직 어렵네요 ㅜㅜ 나중에 읽어보면서 더 공부해봐야겠어요
감사합니당🙂
| @BeforeEach | ||
| void setUp() { | ||
| boardId = boardDao.save(); | ||
| } |
There was a problem hiding this comment.
기론의 두 번째 질문에 대해서 이야기해보죠!
말씀해주신 대로 테스트에서 운영 db를 사용하기에 문제가 발생하죠. 운영과 테스트 db를 분리해야 한다는 생각을 하셨다는 점이 좋네요 👍
현업에서는 이와 같은 문제가 발생할 여지를 차단하기 위해 환경을 분리해요. 개발 환경, 운영 환경 이렇게요. 그리고 운영 환경에서는 이미 검증된 실행 파일만으로 배포되기에 테스트를 따로 돌릴 필요가 없어요. 즉 개발 환경에서 테스트를 돌리면 되는거죠.
다시 돌아와서 지금의 상황을 해결하기 위해서는 스프링 프레임워크를 사용할 경우에는 @sql이라는 어노테이션으로 truncate SQL 파일을 만들어서 테스트가 끝날 때 마다 테이블을 초기화 시키는 방법이 있구요. 만일 JPA를 사용하면 EntityManager라는 개념을 활용하여 초기화하기도 해요. 혹은 testcontainers 라는 기술을 활용하여 테스트마다 컨테이너를 새로 띄워줄 수도 있어요. (어려운 내용이 많죠?)
하지만 지금 이를 적용하기에는 상황도 맞지 않고 학습할 내용과도 거리가 멀어지기에 기론이 고민하시는 부분을 dao에 모든 요소를 제거하는 메서드를 만들어서 해결하는 것도 좋겠네요. 물론 제거한다고해서 자동으로 증가하는 id가 초기화되지 않지만 지금으로서는 적절한 해결방법이라 생각되어요.
There was a problem hiding this comment.
오..오... 테스트를 편하게 하기 위한 여러 방법이 많이 있었군요.. 😭
역시 알면 알수록 공부해야 할 양이 많아지는 거 같아요! 신기하면서 재밌네요 ㅎㅎㅎㅎㅎ 감사합니다!
알려주신 키워드들도 공부해봐야겠어요!
우선 @AfterEach로 매 테스트가 끝날 떄, 마다 테스트할때 사용했던 데이터들을 날려주었습니다!
- 게임 재시작되었을 때, 메시지 한글로 수정
| // 나중에 방 생성하고 게임 만들 때, 이용 | ||
| // public Board createGame() { | ||
| // Long boardId = boardDao.save(); | ||
| // Pieces pieces = Pieces.createInit(); | ||
| // Board board = Board.create(pieces, Turn.init()); | ||
| // | ||
| // pieceDao.save(pieces.getPieces(), boardId); | ||
| // return board; | ||
| // } |
There was a problem hiding this comment.
주석이 존재하네요!
개발을 위해 남겨두셨다고 생각되지만 주석은 코드를 읽는 데에 방해가 되어요. 구현을 놓치고 빼먹으면 누구도 관리하지 않는 레거시가 될 수도 있구요.
코드가 아쉬울 수 있겠지만 깔끔한 코드와 원활한 협업을 위해 지워도 좋겠어요 😉
|
|
||
| ## 4, 5단계 | ||
|
|
||
| ### api 명세 |
There was a problem hiding this comment.
고생하셨어요! 회사에 들어와보니 문서를 잘 작성하는 것도 능력이더라구요! 지금부터 조금씩 연습해보죠!
추가로 API 명세는 서버쪽에서 담당해야하는 책임이기에 읽기 좋게 만들어주면 협업할 때에 도움이 됩니다!
There was a problem hiding this comment.
좋은 말씀 감사합니다...진짜 최고에요 토니!
경험과 함께 말씀해주시니깐 완전 동기부여가 되네요😃
감사합니당
src/main/java/chess/board/Team.java
Outdated
| if ("black".equals(name)) { | ||
| return Team.BLACK; | ||
| } | ||
| if ("white".equals(name)) { | ||
| return Team.WHITE; | ||
| } | ||
| return Team.NONE; | ||
| } | ||
|
|
||
| public String value() { | ||
| if (this.equals(Team.BLACK)) { | ||
| return "black"; | ||
| } | ||
| if (this.equals(Team.WHITE)) { | ||
| return "white"; | ||
| } | ||
| if (this.equals(Team.NONE)) { | ||
| return "none"; | ||
| } |
There was a problem hiding this comment.
black, white, none와 같이 하드코딩 되어 있는 값을 관리해줄 수 없을까요?
There was a problem hiding this comment.
토니! 리펙토링 중 궁금한 점이 생겨서 질문드립니다! Team 과 PieceFactory모두에서 공통으로 발생하는 궁금증인데요!
위의 사진에서 enum이 관리하는 상수값인 대문자 이름들이ex) BLACK, WHITE, NONE 결국엔 소문자 이름ex)black, white, none들로 디비에 저장되고 있습니다. 이때 위 사진처럼 toLowerCase()로 소문자로 만들어서 관리하는 게 좋을까요? 아니면 아래처럼(아래는 PieceFactory클래스입니다!) 소문자 이름을 type과 같은 속성(변수)으로 가지고있는 게 좋을까요?
저는 상수 자체가 그 팀을 나타내는 이름이니깐 그냥 toLowerCase를 써도 될것 같다고 생각했었습니다. 또한 단지 소문자로 변환만한다고 생각했고 괜히 변수로 갖고 관리하면 변동이 생길 때 두 가지 모두 수정해야한다고 생각했습니다. 예를 들면 BLACK과 black 모두 수정해야하는 것 처럼요!
토니는 어떻게 생각하시나요..?
There was a problem hiding this comment.
저도 toLowerCase로 관리하면 기존의 ROOK과 같은 이넘에 선언된 명칭을 변환하는거기에 변수로 관리하는 경우보다 관리포인트가 줄어들거 같네요 😬
| if (type.equals("rook")) { | ||
| return new Rook(loadPosition, Team.from(team)); | ||
| } | ||
| if (type.equals("knight")) { | ||
| return new Knight(loadPosition, Team.from(team)); | ||
| } | ||
| if (type.equals("bishop")) { | ||
| return new Bishop(loadPosition, Team.from(team)); | ||
| } | ||
| if (type.equals("queen")) { | ||
| return new Queen(loadPosition, Team.from(team)); | ||
| } | ||
| if (type.equals("king")) { | ||
| return new King(loadPosition, Team.from(team)); | ||
| } | ||
| if (type.equals("pawn")) { | ||
| return new Pawn(loadPosition, Team.from(team)); | ||
| } | ||
| if (type.equals("empty")) { | ||
| return new Empty(loadPosition); | ||
| } |
There was a problem hiding this comment.
지금과 같은 상황이라면 체스 말이 추가되거나 제거되면 if문이 계속 변경되겠군요. 더 적절한 방법으로 Piece를 생성해줄 수는 없을까요?
There was a problem hiding this comment.
함수형인터페이스를 이용해서 구현해 봤습니다! 적절하게 사용했는지 모르겠네요😂
|
|
||
| .centered { | ||
| text-align: center; | ||
| /*background-image: url("../images/titleBackground.jpg");*/ |
There was a problem hiding this comment.
해당 이미지는 사용되지 않네요~
사용하지 않는 경우에는 꼭 제거하고 pr을 올려주시면 좋아요
그 이유는 이 코드가 남아있는 상태에서 몇 년 후에 다른 사람이 이 코드로 추가 개발을 진행할 때에 이전에 개발한 사람이 어떤 의도를 가지고 주석을 남겼을지 모르기에 100% 확신을 가지고 지우지 못하기 때문이에요 😬
There was a problem hiding this comment.
잠깐 주석 처리해두고 나중에 지운다는 게 까먹었네요!😅
이 코드가 남아있는 상태에서 몇 년 후에 다른 사람이 이 코드로 추가 개발을 진행할 때에 이전에 개발한 사람이 어떤 의도를 가지고 주석을 남겼을지 모르기에 100% 확신을 가지고 지우지 못하기 때문이에요
이해가 확 가네요! 좋은 말씀 감사합니당
| } | ||
| } | ||
|
|
||
| private static class FakePiece { |
There was a problem hiding this comment.
FackPiece보다 MockPiece가 나은 이름이라 생각되네요~!
|
|
||
| import static spark.Spark.get; | ||
| import static spark.Spark.port; | ||
| import static spark.Spark.staticFileLocation; | ||
|
|
||
| public class WebApplication { |
안녕하세요 토니!
오랜만이네요 ㅎㅎ...
크게 html, css, js랑 Dao 테스트에서 막히는 부분이 많아서 오래 걸렸네요 😂
이번 단계를 진행하면서 궁금한 점에 크게 2가지 있었습니다!🤔
Board board를 받아 dto로 변환하는 것보단(왜냐하면 dto는 값을 전달하는 역할만 갖도록 하기 위해서입니다.)controller에서convertToDto라는 메서드를 만들어서 해주었는데 이게 맞는 방법인지 궁금합니다!왜냐하면 지금은 dto개수가 적은데 나중에 프로젝트가 커지면 이러한 dto변환 로직들이 많아 질텐데 매번 controller단에서 변환해주면 controller가 너무 비대해지고 dto 관리도 어렵다고 생각되기 떄문입니다!
아! 그리고 아직은 선택 요구사항의 게임 방은 만들지 않았습니다! 피드백을 듣고 진행해보려구 합니다!
감사합니다!