refactor: startMatch 메서드 수정 및 검증 추가 #488
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…chStrategy 내부로 옮겼습니다. 또한 startMatch 를 하기 위한 조건을 추가했습니다. LeagueAt 전에는 match 를 시작할 수 없습니다.
I-migi
reviewed
Nov 28, 2024
| @@ -169,7 +169,7 @@ public CommonResponse<String> startMatch( | |||
| StartMatchCommand startMatchCommand = | |||
| new StartMatchCommand(clubToken, leagueId, matchId); | |||
| MatchOperationHandler matchOperationFacade = matchFacade.getMatchOperationHandler(leagueId); | |||
| matchOperationFacade.startMatch(startMatchCommand); | |||
| matchOperationFacade.startMatch(leagueId, startMatchCommand); | |||
There was a problem hiding this comment.
startMatchCommand record 안에 leagueId가 있는데 leagueId 를 추가로 넣은 이유가 있나요?
There was a problem hiding this comment.
Command 안에 leagueId가 있었군요! command 객체 내부에 있는 leagueId를 사용하겠습니다!
| public void startMatch(MatchStrategy matchStrategy, Long leagueId, Long matchId) { | ||
| League league = leagueReader.readLeagueById(leagueId); | ||
| if (LocalDateTime.now().isBefore(league.getLeagueAt())) { | ||
| throw new CannotStartMatchException(league.getLeagueId(), league.getLeagueAt()); |
There was a problem hiding this comment.
Exception의 이름을 리그 시작 시간 전이라는 의미를 가지게 만드는게 좋을 것 같아요! 😄
There was a problem hiding this comment.
넵 반영해서 수정하겠습니다!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR에 대한 설명 🔍
startMatch 메서드의 중복되는 코드를 해결하기 위해 AbstractMatchStrategy 내부로 옮겼습니다. 또한 startMatch 를 하기 위한 조건을 추가했습니다. LeagueAt 전에는 match 를 시작할 수 없습니다.
변경된 사항 📝
FreeSinglesMatchStrategy,TournamentSinglesMatchStrategy의 startMatch 메서드가 동일한 문제를 해결했습니다.해당 메서드를
AbstractSinglesMatchStrategy로 옮겼습니다.Doubles Match 의 경우에도 동일한 문제가 있어
AbstractSinglesMatchStrategy로 옮겼습니다.현재 시간이 LeagueAt 이전이라면 예외를 발생합니다.
CannotStartMatchException에러 메시지는 아래와 같습니다.
PR에서 중점적으로 확인되어야 하는 사항