[3, 4단계 - 체스] 구름(김민수) 미션 제출합니다.

[alstn113]

안녕하세요. 스티치! 구름 ⛅️입니다.
1,2단계 PR를 머지한지 벌써 일주일이 넘었네요.
핑계지만 글쓰기와 테크톡때문에 정신이 없었습니다.

코드가 깔끔하지 않습니다. 그럼에도 불구하고 피드백을 받을 수 있는 기간이 얼마 남지 않은 것 같아서 PR을 빠르게 보내봅니다.

우선 저는 게임을 저장할 때 chess_game을 만들고, 그것과 연결된 기보들을 저장합니다.
게임이 끝난 경우는 chess_game의 game_status를 finished로 변경합니다.
기존에 게임이 있는지 찾는 방법은 chess_game을 id로 정렬하고 game_status가 playing인 마지막 하나를 가져옵니다.
그리고 도메인인 chessGame에서 load를 통해 이전 기보들을 실행시킵니다.

dao테스트 방법은 마땅히 생각나지 않아서 @AfterEach를 통해서 테이블을 지우는 방법을 선택했습니다.

피드백 주시면 빠르게 반영하겠습니다. 죄송합니다.

[lxxjn0]

구름, 3,4단계 미션 구현하시느라 수고하셨습니다.

이번 단계도 마찬가지로 정말 적절한 정도 내에서 코드 구현을 너무 잘 해주신 것 같아요. 제가 특별하게 피드백을 드릴만한 부분이 잘 보이질 않네요.. ㅠㅠ 테스트도 적절하게 잘 작성해주신 것 같고요. 우선 간단한 부분에 대해서 피드백을 남겨봤습니다. 혹시나 추가로 의견을 받거나 피드백을 받고 싶은 부분 또는 고민이 되는 부분이 있다면 참고해서 추가적인 피드백을 드려보도록 하겠습니다!! 👍🏼

[lxxjn0]

이번에 필드에서 direction과 pieceType을 제거하였는데 이유가 있을까요?

[alstn113]

이게 참 어렵네요.
필드로 가지고 있는 것보다 좋은 것 같아서 여러 코치님들한테 물어봤었거든요.
개인적으로 부모 클래스의 필드로 가지고 있으면 부모 클래스가 알아야할 부분이 늘어난다고 생각했어요.

스티치는 어떻게 생각하시나요? 어떤 느낌인지는 알겠는데 표현하기가 힘드네요.

[lxxjn0]

전 메서드로 제공되는 것들은 행위인 인터페이스라고 생각하는데 체스의 기물에서 방향과 기물 유형은 행위로 보기 보다는 속성으로 보는게 맞을 것 같다고 생각했습니다. 그래서 그 성격에 맞게 필드로 가지는게 더 낫지 않나? 라는 생각을 하긴 했어요. 프로퍼티는 결국 객체의 존재에 대한 표현이라고 생각하는데 이 두 값은 기물이라는 객체를 표현할 때 필요한 속성이 아닐까? 라고 생각하긴 합니다 :) 개발자 개인의 차이가 있을 수 있으니 참고만 해주세요!

[alstn113]

음.. 우선 chess game 테이블과 dao를 제거했습니다.
추가 기능 구현 목록인 유저와 방이 없는데 chess game을 사용해서 게임 기록같은 것들을 만드는 것은 우선 불필요하다고 생각했습니다.
현재 게임은 id, source, target을 가지는 move 테이블이 있습니다. 텅 비어 있으면 새로운 게임을 하고 move가 있으면 불러와서 이전 게임을 계속합니다. 왕이 죽을 경우 move를 모두 지우고 다음 게임은 새롭게 시작할 수 있습니다.

다음으로 말씀하신 chessGame에 대해서입니다.
현재 구조는 controller에서 chessGame을 지우고 service에서 chessGame을 만듭니다. 그리고 run메서드 내부에서 파라미터로 넘기면서 게임을 진행합니다.

아마 피드백에 대한 의도는 getChessGame 혹은 loadChessGame을 메서드를 ChessGameService에 두고 사용하는 방향으로 추정됩니다. (맞나요?? ㅠㅠ)
근데 뭔가 이게 필요있나 잘 모르겠어요. 다른 크루들은 load하는게 비용이 있으니 캐싱도 하고 그러더라구요.
기보를 저장해서 그런가 뭔가 이상해지네요. 기보를 불러올 때도 상태패턴을 사용해서 그런가 복잡합니다.
상태패턴을start -> move -> ... 순으로 만들었어요. 게임 불러오기는 start -> move -> move 반복 으로 작동해요.
그래서 start를 하기 전에 게임을 불러올 수 없어요. 그래서 뭔가 더 불편한 것 같네요.

미리 리뷰 감사드립니다...

[lxxjn0]

구름, 피드백 반영하시느라 수고하셨습니다. 우선 간단한 부분들에 대한 피드백만을 남겨뒀는데 한번 확인 부탁드릴게요.

추가로 남겨주신 코멘트에 대해서는 제 생각은 조금 다르긴 합니다. 먼저 다른 크루들이 캐싱 등을 한다고 했는데 지금은 캐싱을 하는 것보다는 도메인을 잘 설계하고 작성하는 것이 더 중요하다고 생각합니다. 구름이 작성한 코드는 전에도 말씀드렸지만 제가 보기엔 충분히 잘 짜여진 코드라고 생각합니다. 과하지도 않으면서 부족하지도 않고, 지금 집중해야 할 부분에 잘 집중해서 작성한 코드라고 생각해요. 상태패턴도 충분히 적용해볼만한 내용이었다고 생각하고요.

게임도 당연히 start를 하지 않는 이상은 시작되지 않는 것이 맞기에 지금의 코드가 이상한 것은 아니라고 생각합니다. 만약 start 키워드를 입력하기 전에 '이미 존재하는 게임이 있을 경우 게임을 로드한다'라는 요구사항을 구현하고 싶다면 이는 코드 상에서 충분히 해결 가능하다고 생각합니다. 키워드를 입력받기 전 db를 조회한 후, 만약 게임 데이터가 존재하면 내부적으로 start를 호출하면 될 것이고요. 그래서 상태패턴의 문제는 아니지 않을까? 생각해요.

이번에도 좀 다양하게 피드백을 드리고 싶었지만 제가 보기에 구름의 코드는 따로 피드백을 드릴 부분이 없어보여서 더 드리질 못했네요.. 그치만 레벨1 마지막 미션이고 시간도 살짝 여유가 있는 만큼 바로 머지하지 않고 한번 더 RC로 남겨두었습니다. 혹시 궁금한 점이 있거나 좀 구현 상에서 한번 더 이야기를 나누고 싶은 부분이 있다면 DM으로 연락 주세요 :) 만약 추가로 더 피드백 받을 만한 내용이 없다면 말씀주시면 머지 하도록 하겠습니다 👍🏼

미션 진행하시느라 수고 많으셨어요 :)

[lxxjn0]

전 메서드로 제공되는 것들은 행위인 인터페이스라고 생각하는데 체스의 기물에서 방향과 기물 유형은 행위로 보기 보다는 속성으로 보는게 맞을 것 같다고 생각했습니다. 그래서 그 성격에 맞게 필드로 가지는게 더 낫지 않나? 라는 생각을 하긴 했어요. 프로퍼티는 결국 객체의 존재에 대한 표현이라고 생각하는데 이 두 값은 기물이라는 객체를 표현할 때 필요한 속성이 아닐까? 라고 생각하긴 합니다 :) 개발자 개인의 차이가 있을 수 있으니 참고만 해주세요!

[lxxjn0]

체스 게임 도메인에서 기물의 색상을 Color라는 용어로 일관되게 사용하면 코드의 이해도나 통일감에 있어 더 좋을 것 같아요. getCurrentColor와 같이요. 동일한 도메인을 표현하는 용어가 여러가지로 사용된다면 코드를 읽는 사람으로 하여금 혼동을 줄 여지가 있을 것 같아서 제안드려봅니다 :)

[alstn113]

일관성 참 중요하죠.
처음부터 turn을 사용하고 싶었는데 color로 잘못 지었더니 헷갈렸던 것 같습니다.
모두 수정했습니다.