Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[1,2,3단계 - 체스] 춘식(안예장) 미션 제출합니다. #209

Merged
merged 74 commits into from
Mar 27, 2021

Conversation

bimppap
Copy link

@bimppap bimppap commented Mar 22, 2021

안녕하세요 휴!!
1단계 마지막 미션을 함께하게 되었네요. 미션 기간 동안 잘 부탁드립니다!! 🙏

이번 미션은 유독 어렵게 느껴져서 시간이 오래 걸린 것 같네요ㅠㅠ
그래도 페어랑 많은 얘기를 나누면서 새로 배우게 된 점이 많아 즐거웠습니다. 😄

미션을 진행하면서 신경을 많이 부분은 움직이기로 한 말이 갈 수 있는 곳을 가지는 Path 객체예요.
시중의 체스 게임들에서 말을 클릭하면 갈 수 있는 경로가 모두 보인다는 점에 착안해
갈 수 있는 위치를 모두 가진 Path를 만들고, target이 갈 수 있는지 확인하는 방식으로 체스말 이동 기능을 구현했습니다.
가야 하는 모든 위치의 유효성을 확인하는 부분이 어려워 여기서 시간을 많이 쏟은 것 같아요.

궁금한 점은 Command에 관한 것인데, 초반엔 도메인 패키지에 속하는 enum 클래스로 어떤 명령인지만 확인하는 정도로만 구현했습니다.
이 상황에서 구현을 하다보니 Game에서 하나하나 체크해 수행을 하는 상황이 생겨 수행을 하는 부분도 Command 클래스로 옮겼습니다.
그 결과 Command에서 View도 쓰이고 다른 도메인 객체들도 사용하는 것 같아 Controller 패키지로 옮겼는데 이렇게 구현해도 되는 지 의문이 조금 듭니다.
더 좋은 방법이 있다면 부담없이 알려주세요!!! 피드백으로 마구 썰어주시면 감사하겠습니다 🤣

(+) 아직 리팩토링 중인지라 PR이후에도 커밋이 더 올라갈 것 같습니다. 도중에 궁금한 점이 생긴다면 DM 드리겠습니다!!

bimppap and others added 30 commits March 16, 2021 16:43
Copy link

@Hue9010 Hue9010 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

춘식 반가워요! 이번 리뷰를 맡은 휴입니다! 😄
리뷰가 생각보다 늦어진 점 죄송합니다. 😭 🙏
질문에 대한 답변과 개선하면 좋을 부분 피드백 남겼는데 궁금한건 언제든 질문 주세요!

src/main/java/chess/controller/Command.java Outdated Show resolved Hide resolved
src/test/java/chess/domain/result/ResultTest.java Outdated Show resolved Hide resolved
src/main/java/chess/domain/Game.java Show resolved Hide resolved
src/main/java/chess/domain/Game.java Outdated Show resolved Hide resolved
src/main/java/chess/domain/board/Board.java Outdated Show resolved Hide resolved
src/main/java/chess/domain/board/Path.java Outdated Show resolved Hide resolved
src/main/java/chess/domain/board/Board.java Outdated Show resolved Hide resolved
src/main/java/chess/domain/position/Column.java Outdated Show resolved Hide resolved
src/main/java/chess/domain/board/Paths.java Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
@bimppap
Copy link
Author

bimppap commented Mar 26, 2021

안녕하세요 휴!!
피드백 반영해서 리팩토링 해봤습니다.
크게 바뀐 점은 다음과 같아요.

  • 커맨드 패턴 적용
    • Controller에서만 View가 쓰이도록 함
    • 기존의 Command(현 Commands)는 찾는 Command를 반환하도록 함
  • Path의 역할 명시화
    • Piece에서 Path의 경로를 계산하도록 함
    • Paths 삭제
    • 특정 체스말끼리의 공통점을 찾아 Piece를 상속받는 Umlimited / Limited 추상 클래스 생성.
      체스말들은 해당하는 추상 클래스를 상속받도록 변경

이번 리팩토링은 많이 힘들었지만 🤣 그만큼 많이 배우게 된 것 같아 뿌듯합니다.
늦은 시간에 디엠 드렸는데도 친절하게 답변 주셔서 너무 감사해요 👍 👍
더 피드백 주실 부분이 있다면 매우 환영입니다!!!

(+) 테스트 코드는 천천히 추가할 예정입니다 :3

Copy link

@Hue9010 Hue9010 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고민하시던 부분 걱정하시던 것과 다르게 잘해주셨네요. 😄 👍👍
이번 단계는 여기서 머지하도록 할게요.
이번엔 크게 피드백 드릴 부분이 없었는데 궁금한건 언제든 질문 주세요. 😉

RefreshState();
}

private void RefreshState() {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RefreshState 😉

@Hue9010 Hue9010 merged commit c9a4eca into woowacourse:rinsabbit Mar 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants