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

[또링] 웹 기반 체스 게임 4단계 구현 #163

Merged
merged 8 commits into from
May 1, 2020

Conversation

jnsorn
Copy link

@jnsorn jnsorn commented Apr 30, 2020

잘 부탁드려요! 😀👍

Copy link

@ttkmw ttkmw left a comment

Choose a reason for hiding this comment

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

안녕하세요~ 임루트입니다.
마지막 미션도 멋지게 구현하셨네요~!
몇가지 개인적인 의견을 남겨봤습니다~
고생하셨습니다~~

import java.util.List;

public class SavedGameBundleDto {
private List<SavedGameDto> room;
Copy link

Choose a reason for hiding this comment

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

타입과 변수명이 다르다보니 조금 헷갈렸습니다..!
둘다 SavedGame(SagedGameDto, sagedGame) 혹은 둘다 room(RoomDto, room) 으로 하는 건 어떨까요??

Copy link
Author

Choose a reason for hiding this comment

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

좋은 의견 고마워요! 모두 일관성 있게 Room으로 수정했어요 😊

List<Game> findAll();

@Query("SELECT ID FROM GAME")
List<Long> findAllById();
Copy link

Choose a reason for hiding this comment

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

findAllById 함수를 보니, id를 활용하여 Game Entity를 가져올 것 같다는 생각이 들었습니다..! id가 where절에 들어갈 것 같은 느낌입니다
아마 findAll()이 id로 game entity를 가져올텐데요,
현재 또링이 원하시는 건 id들을 찾는 것이니 findAllIds()는 어떨까요?

Copy link
Author

Choose a reason for hiding this comment

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

엇..! 원래 findAllId가 메서드 명이었는데 finAllId로 되어있어 오타를 수정하다가 정신없이 ById를 넣은 것 같아요..! 해당 부분 수정하겠습니다😉

}

private Map<String, Double> getScore(ChessGame chessGame) {
Map<Team, Double> score = Score.calculateScore(chessGame.getPieces(), Team.values());
Copy link

Choose a reason for hiding this comment

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

이 함수에서 최종적으로 필요하신 것은 Map<String, Double> 형식인 것 같습니다. 그런데 score가 Map<Team, Double> 형식으로 돼있다보니, 조금 헷갈렸습니다 ㅜㅜ

Score score = Score.calculateScore(chessGame.getPieces(), Team.values());
return ScoreConverter.convert(score);

이건 어떨까요?? 아니면,

Score score = chessGame.calculateScore();
return ScoreConverter.convert(score);

이것도 가능할 것 같습니다!!

Copy link
Author

Choose a reason for hiding this comment

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

오! 저는 생각치도 못한 부분이네요. 변수명이 score이고 map이 team과 double로 이루어져있기 때문에 제가 표현하고자 하는 바는 다 표현했다고 생각했는데, 다른 사람이 읽을 때에는 그런 느낌이군요....! 제안해주신 부분에 대해서 충분히 고민해보도록 할게요.🙂🤔🙂 감사합니다~

@woowahan-pjs woowahan-pjs merged commit cb9d911 into woowacourse:jnsorn May 1, 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.

None yet

3 participants