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주차 미션 제출합니다. #63

Merged
merged 85 commits into from
Apr 7, 2020

Conversation

jnsorn
Copy link

@jnsorn jnsorn commented Mar 26, 2020

안녕하세요! 리뷰를 받게 된 또링이라고 합니다. 아직 모르는 부분이 많은데, 많이 지적해주시면 열심히 고쳐보겠습니다!😁 잘 부탁드립니다.

jnsorn added 30 commits March 24, 2020 19:50
Copy link

@kang-hyungu kang-hyungu left a comment

Choose a reason for hiding this comment

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

또링 피드백 반영 잘 하셨어요 👏
좀더 수정해보면 좋을 부분에 피드백 남겼어요
확인해보시고 궁금한 점 생기면 코멘트 남기거나 dm 주세요

src/main/java/view/OutputView.java Outdated Show resolved Hide resolved

public static void printChessBoard(BoardGame board) {
List<Rank> ranks = board.getBoard().getRanks();
for (int rankIndex = ranks.size() - 1; rankIndex >= 0; rankIndex--) {

Choose a reason for hiding this comment

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

board에서 랭크 반환할 때 역순서로 만들어서 반환해도 되겠네요

Copy link
Author

Choose a reason for hiding this comment

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

🎤 좋은 지적 감사합니다! 한 가지 궁금한 게 있습니다.
board의 역순이 필요한 경우는 출력하는 경우밖에 없는데, 이런 경우에도 view를 위해 도메인에서 역순서로 된 객체를 반환하는 메서드를 만드는 것이 더 좋을까요?

Choose a reason for hiding this comment

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

view 전용 기능을 추가한다기 보다 Rank 리스트를 역순으로 반환하는 기능 추가라고 생각하면 되겠죠

src/main/java/view/OutputView.java Outdated Show resolved Hide resolved
src/main/java/domain/command/MoveInfo.java Outdated Show resolved Hide resolved
src/main/java/domain/command/MoveInfo.java Outdated Show resolved Hide resolved
src/main/java/domain/board/Rank.java Outdated Show resolved Hide resolved
src/main/java/domain/piece/Pawn.java Outdated Show resolved Hide resolved
src/main/java/domain/piece/Pawn.java Outdated Show resolved Hide resolved
src/main/java/domain/piece/Piece.java Outdated Show resolved Hide resolved
src/main/java/domain/piece/Queen.java Outdated Show resolved Hide resolved
Copy link

@kang-hyungu kang-hyungu left a comment

Choose a reason for hiding this comment

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

또링 피드백 반영 잘 하셨어요 👏
몇 가지 간단히 남긴 피드백은 다음 미션에서 적용해보아요
pr은 머지 처리할게요 수고하셨어요

src/main/java/domain/Score.java Show resolved Hide resolved
src/main/java/domain/Score.java Show resolved Hide resolved
src/main/java/domain/Score.java Show resolved Hide resolved
src/main/java/domain/Score.java Show resolved Hide resolved
@kang-hyungu kang-hyungu merged commit bd16176 into woowacourse:jnsorn Apr 7, 2020
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.

2 participants