[1, 2, 3단계 - 체스] 레넌(조형래) 미션 제출합니다.#308
Conversation
choihz
left a comment
choihz
left a comment
There was a problem hiding this comment.
안녕하세요, 레넌! 체스 미션을 함께하게 된 리뷰어 코니입니다. 👋
미션을 잘 구현해 주셨네요 👍 코멘트 보시고 더 궁금한 점이나 의견이 있으시다면 언제든 코멘트 또는 DM 주세요~
src/main/java/chess/domain/piece/generator/NormalPiecesGenerator.java
Outdated
Show resolved
Hide resolved
choihz
left a comment
choihz
left a comment
There was a problem hiding this comment.
안녕하세요, 레넌!
수정해 주신 내용을 확인하고 피드백을 남겼는데, 사실 피드백 내용들이 다 큰 수정을 필요로 할 것 같아요. 다음 단계를 진행하면서 함께하기 어려울 듯한 부분과 고민이 드는 지점 등은 언제든 DM 또는 코멘트로 공유해 주세요.
| @@ -0,0 +1,54 @@ | |||
| package chess.domain.piece.fixedmovablepiece; | |||
There was a problem hiding this comment.
패키지의 이름을 바꾸는 것이 좋을까요?? 아니면 굳이 분리할 필요없이 piece 패키지에 한번에 넣어서 관리하는 것이 좋을까요?? 개인적인 생각으로는 한번에 관리하기에는 너무 많은 클래스들이 있어 보기 어려울 것 같아 분리하는 것이 좋다고 생각합니다. 적당한 패키지 이름이 떠오르지가 않습니다ㅜㅜ
말씀하신 것처럼, 하나의 패키지 안에 너무 많은 클래스가 있으면 관리가 몹시 어려워 적절히 분류하는 게 중요합니다. 적절한 분류와 적절한 이름 모두 너무나 중요한데요, 지금 구조는 살짝 아쉬운 부분이 있는 것 같아요. 물론 이런 부분 역시 정답이 없기 때문에 어렵죠 🥲
우선 fixedmovablepiece, generator, pawn, straightmovablepiece 패키지가 동일 레벨인 상황인데 직관적이지 못한 느낌이고, Color, EmptyPiece, Piece, Symbol 역시 piece 바로 하위에 위치하고 있는데 조금 더 고민이 필요해 보입니다.
또한 처음에 드린 코멘트는 fixedmovablepiece라는 패키지 이름에 관한 것이었는데요, 패키지 이름을 이렇게 길게 지으면 가독성이 현저하게 떨어질 수밖에 없죠. piece 패키지 하위에 있는데 이름에 굳이 piece가 들어갈 필요는 없어 보여요. 그리고 사실 근본적으로는 FixedMovablePiece, StraightMovablePiece라는 클래스 이름 자체가 좀 아쉽습니다.
| import java.util.List; | ||
| import java.util.Map; | ||
|
|
||
| public abstract class FixedMovablePiece extends Piece { |
There was a problem hiding this comment.
어떻게 King과 Knight가 함께 묶이고 Bishop, Queen, Rook이 함께 묶이는지, FixedMovablePiece와 StraightMovablePiece라는 이름을 함께 두고 봐도 이해하기 어려웠습니다. FixedMovablePiece, StraightMovablePiece는 실제로 한 줄 빼고 동일한 것 같은데, 구조를 개선해 보시면 좋을 것 같아요.
2 participants
안녕하세요, 코니!
체스미션이 이전 미션들보다 훨씬 어려워서 많이 헤맸는데요.
그래도 열심히 코드를 짰으니, 잘 부탁드리겠습니다!