Conversation
Summary of ChangesHello @TAEW00KIM, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 Pull Request는 매니저가 API를 통해 게임 상태를 'FINISHED'로 직접 변경할 때 경기 결과가 올바르게 저장되지 않던 문제를 해결합니다. 기존에는 서버 스케줄러를 통해 게임이 종료될 때만 결과가 저장되었으나, 이제는 직접 종료 API 경로에서도 스케줄러 종료와 동일하게 경기 결과가 저장되도록 하여 게임 데이터의 일관성을 보장합니다. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
이번 PR은 매니저가 API를 통해 직접 게임을 종료시킬 때 경기 결과가 저장되지 않는 문제를 해결하셨네요. GameService.updateGame 메소드에서 게임 상태가 FINISHED로 변경될 때 determineResult를 호출하도록 로직을 추가하여 문제를 잘 해결했습니다. 추가된 테스트 코드 게임을_직접_종료하면_결과가_저장된다도 변경된 로직을 잘 검증하고 있습니다. 한 가지 제안 사항으로, 결과 결정 조건에 대한 로직을 Game 도메인 엔티티로 옮겨 캡슐화를 강화하는 방안을 검토해보시면 좋겠습니다. 자세한 내용은 코드 리뷰 코멘트를 참고해주세요.
| private void updateResultIfFinished(Game game, GameState state) { | ||
| if (!GameState.FINISHED.equals(state)) { | ||
| return; | ||
| } | ||
| if (game.getGameTeams().size() != Game.MINIMUM_TEAMS) { | ||
| return; | ||
| } | ||
| game.determineResult(); | ||
| } |
There was a problem hiding this comment.
경기 결과를 계산할 수 있는지 판단하는 로직(참여 팀이 2개인지 확인)은 애플리케이션 서비스 레이어보다는 Game 도메인 엔티티에 속하는 것이 더 적절해 보입니다.
도메인 주도 설계(DDD) 원칙에 따라 도메인 객체의 캡슐화를 강화하고 책임 분리를 명확히 하기 위해, 이 로직을 Game 엔티티 내부로 옮기는 것을 제안합니다.
Game 클래스에 tryDetermineResult()와 같은 새로운 메서드를 만들어 관련 로직을 포함시키고, GameService에서는 이 메서드를 호출만 하도록 변경하면 좋을 것 같습니다.
Game.java에 추가할 메서드:
public void tryDetermineResult() {
if (getGameTeams().size() == MINIMUM_TEAMS) {
determineResult();
}
}이렇게 변경하면 Game 엔티티가 자신의 비즈니스 규칙을 스스로 책임지게 되어, 코드가 더 유지보수하기 좋고 견고해질 것입니다.
private void updateResultIfFinished(Game game, GameState state) {
if (GameState.FINISHED.equals(state)) {
game.tryDetermineResult();
}
}
There was a problem hiding this comment.
저도 이건 Game 내부로 들어가도 될 것 같아요!
그리고 updateResult 로 둬서 캡슐화를 지켜도 될 것 같은데 어떠세요?
Game 이 업데이트 할 수 있는 상태인지 아닌지는 서비스 레이어에서는 관여하지 말고, 도메인 레이어에서만 알고 있는 게 더 적절할 것 같다는 생각이 들어요.
| private void updateResultIfFinished(Game game, GameState state) { | ||
| if (!GameState.FINISHED.equals(state)) { | ||
| return; | ||
| } | ||
| if (game.getGameTeams().size() != Game.MINIMUM_TEAMS) { | ||
| return; | ||
| } | ||
| game.determineResult(); | ||
| } |
There was a problem hiding this comment.
저도 이건 Game 내부로 들어가도 될 것 같아요!
그리고 updateResult 로 둬서 캡슐화를 지켜도 될 것 같은데 어떠세요?
Game 이 업데이트 할 수 있는 상태인지 아닌지는 서비스 레이어에서는 관여하지 말고, 도메인 레이어에서만 알고 있는 게 더 적절할 것 같다는 생각이 들어요.
2 participants
이슈 배경
기존의 문제
FINISHED처리하면 결과가 저장되지 않는 문제가 있었습니다.해결 방식
GameService.updateGame에서 상태 변경 값을 먼저 파싱한 뒤, 상태가FINISHED이고 참가 팀이 2팀인 경우determineResult()를 수행하도록 보완했습니다.확인해야할 부분