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

feat: 토너먼트 복식 부전승 로직 구현 #526

Merged
merged 4 commits into from
Dec 5, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -133,4 +133,8 @@ public Integer getSetNumberInProgress() {
}
return getSetInProgress().get().getSetNumber();
}

public void byeMatch() {
this.matchStatus = MatchStatus.BYE;
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.badminton.infrastructure.match.reader;

import java.util.Arrays;
import java.util.List;

import org.badminton.domain.common.enums.MatchStatus;
Expand Down Expand Up @@ -64,7 +65,8 @@ public boolean allMatchesFinishedForLeague(Long leagueId) {

@Override
public boolean allMatchesNotStartedForLeague(Long leagueId) {
return doublesMatchRepository.allMatchesNotStartedForLeague(leagueId);
List<MatchStatus> statuses = Arrays.asList(MatchStatus.NOT_STARTED, MatchStatus.BYE);
Copy link
Collaborator

Choose a reason for hiding this comment

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

이 두가지 Status가 어떤 상태인지 변수명에 더 드러나면 좋을 것 같아요.

return doublesMatchRepository.allMatchesNotStartedForLeague(leagueId, statuses);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,9 @@ boolean areAllMatchesFinishedOrBye(@Param("leagueId") Long leagueId, @Param("rou
@Query("SELECT COUNT(m) = 0 FROM DoublesMatch m WHERE m.league.leagueId = :leagueId AND m.matchStatus != 'FINISHED'")
boolean allMatchesFinishedForLeague(@Param("leagueId") Long leagueId);

@Query("SELECT COUNT(m) = 0 FROM DoublesMatch m WHERE m.league.leagueId = :leagueId AND m.matchStatus != 'NOT_STARTED'")
boolean allMatchesNotStartedForLeague(@Param("leagueId") Long leagueId);
@Query("SELECT NOT EXISTS (SELECT 1 FROM DoublesMatch match WHERE match.league.leagueId = :leagueId AND match.matchStatus NOT IN (:statuses))")
boolean allMatchesNotStartedForLeague(@Param("leagueId") Long leagueId,
@Param("statuses") List<MatchStatus> statuses);

}

Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@
import java.util.List;

import org.badminton.domain.common.enums.MatchResult;
import org.badminton.domain.common.enums.MatchStatus;
import org.badminton.domain.common.enums.SetStatus;
import org.badminton.domain.common.exception.match.AlreadyWinnerDeterminedException;
import org.badminton.domain.common.exception.match.ByeMatchException;
import org.badminton.domain.common.exception.match.LeagueParticipantsNotExistsException;
import org.badminton.domain.common.exception.match.PreviousDetNotFinishedException;
import org.badminton.domain.common.exception.match.RoundNotFinishedException;
Expand Down Expand Up @@ -57,6 +59,14 @@ private static boolean isMatchWinnerDetermined(DoublesMatch doublesMatch) {
|| doublesMatch.getTeam2MatchResult() == MatchResult.WIN;
}

private static boolean isParticipantOddSize(List<LeagueParticipant> participants) {
return participants.size() % 4 != 0;
}

private static boolean isDoubleMatchOddSize(List<DoublesMatch> doublesMatches) {
return doublesMatches.size() % 2 != 0;
}

@Override
public BracketInfo makeBracket(League league, List<LeagueParticipant> leagueParticipantList) {
List<DoublesMatch> allMatches = new ArrayList<>();
Expand All @@ -69,6 +79,10 @@ public BracketInfo makeBracket(League league, List<LeagueParticipant> leaguePart
allMatches.addAll(createFirstRoundMatches(league, currentParticipants));
allMatches.addAll(createSubsequentRoundsMatches(league, totalRounds));

allMatches.stream()
.filter(doublesMatch -> doublesMatch.getMatchStatus() == MatchStatus.BYE && doublesMatch.getTeam1() != null)
Copy link
Collaborator

Choose a reason for hiding this comment

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

doublesMatch.getTeam1()이 null이 아니어야 하는 조건을 체크하는 이유가 무엇일까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

만약 doublesMatch.getTeam1()이 null이라면, 다음 라운드로 승자를 전달할 수 없어 정상적인 대진표 업데이트가 불가능합니다. 이를 사전에 방지하기 위해 조건을 확인합니다

Copy link
Collaborator

Choose a reason for hiding this comment

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

Team Entity에 getEmptyTeam() 등을 추가해서 null이 직접적으로 return 되지 않도록 하면 더 좋을 것 같습니다! 지금은 null이 담고 있는 의미가 없어서 무슨 목적인지 명확하지 않은 것 같아요. 추후 리팩토링 시 고려해주세요!

.forEach(this::updateNextRoundMatch);
Copy link
Collaborator

Choose a reason for hiding this comment

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

이거 다음 라운드 매치에 Participant 업데이트하는 메서드일까요? 만약 그렇다면, updateNextRoundMatchParticipant가 어떨까요? 지금은 다음 라운드 매치를 어떻게 업데이트하는 것인지 조금 알기 어려운 것 같아요


return BracketInfo.fromDoubles(totalRounds, allMatches);
}

Expand All @@ -82,6 +96,10 @@ public SetInfo.Main registerSetScoreInMatch(Long matchId, Integer setNumber,

validatePreviousRoundCompletion(doublesMatch.getLeague().getLeagueId(), doublesMatch.getRoundNumber());

if (doublesMatch.getMatchStatus() == MatchStatus.BYE) {
throw new ByeMatchException(doublesMatch.getId());
Copy link
Collaborator

Choose a reason for hiding this comment

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

RegisterScoreInByeMatchException 어떨까요? 부전승이면 무조건 예외가 발생하는 것인가? 라는 생각이 들었어요

Copy link
Collaborator

Choose a reason for hiding this comment

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

또 match type도 넘겨주면 좋을 것 같습니다! SINGLES/DOUBLES 인지도 넘어가야 할 것 같아요

}

if (isMatchWinnerDetermined(doublesMatch)) {
throw new AlreadyWinnerDeterminedException(doublesMatch.getId());
}
Expand All @@ -101,8 +119,7 @@ public SetInfo.Main registerSetScoreInMatch(Long matchId, Integer setNumber,
}

if (isMatchWinnerDetermined(doublesMatch)) {
doublesMatchStore.store(doublesMatch);
updateNextRoundMatch(doublesMatch);
processMatchAndNextRound(doublesMatch);
}
if (isAllMatchFinished(doublesMatch)) {
leagueReader.readLeagueById(doublesMatch.getLeague().getLeagueId()).finishLeague();
Expand Down Expand Up @@ -155,36 +172,92 @@ public Main retrieveSet(Long matchId, int setNumber) {
return SetInfo.fromDoublesSet(matchId, setNumber, doublesSet);
}

private void processMatchAndNextRound(DoublesMatch doublesMatch) {
doublesMatchStore.store(doublesMatch);
updateNextRoundMatch(doublesMatch);
boolean allRoundMatchesDone = doublesMatchReader.allRoundMatchesDone(doublesMatch.getLeague().getLeagueId(),
doublesMatch.getRoundNumber());
boolean currentRoundInTotalRound =
Copy link
Collaborator

Choose a reason for hiding this comment

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

boolean 타입은 앞에 is 를 붙인 네이밍 컨벤션을 적용해보는게 어떨까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

복수형일 때는 boolean 값 앞에 is를 안붙이는 거로 알고 있는데 is 를 붙여야 하나요?

doublesMatch.getRoundNumber() < doublesMatch.getLeague().getTotalRounds();

if (allRoundMatchesDone && currentRoundInTotalRound) {
List<DoublesMatch> nextRoundMatches = doublesMatchReader.findMatchesByLeagueAndRound(
doublesMatch.getLeague().getLeagueId(), doublesMatch.getRoundNumber() + 1
);
nextRoundMatches.stream()
.filter(match -> match.getMatchStatus() == MatchStatus.BYE)
.forEach(this::updateNextRoundMatch);
}
}

private List<DoublesMatch> createFirstRoundMatches(League league, List<LeagueParticipant> participants) {
List<DoublesMatch> matches = new ArrayList<>();
for (int i = 0; i < participants.size(); i += PARTICIPANTS_PER_TEAM * TEAMS_PER_MATCH) {

for (int i = 0; i < participants.size() - 2; i += PARTICIPANTS_PER_TEAM * TEAMS_PER_MATCH) {
Team team1 = new Team(participants.get(i), participants.get(i + 1));
Team team2 = new Team(participants.get(i + 2), participants.get(i + 3));
DoublesMatch match = new DoublesMatch(league, team1, team2, 1);
makeSetsInMatch(match);
doublesMatchStore.store(match);
matches.add(match);
}

if (isParticipantOddSize(participants)) {
Team byeTeam = new Team(participants.remove(participants.size() - 1),
participants.get(participants.size() - 2));
Comment on lines +206 to +208
Copy link
Collaborator

Choose a reason for hiding this comment

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

participants.size에서 1을 빼는 이유와 2를 빼는 이유가 와닿지 않아요. 상수로 빼는게 어떨까요?

Copy link
Collaborator

Choose a reason for hiding this comment

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

remove를 하는 목적은 뭘까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

remove 뜻은 participant 에서 해당 인덱스의 값을 제거 후 get 하는 것입니다. get을 쓰지 않고 remove 를 쓰는 이유는
remove를 사용하면 "해당 참가자는 이후 로직에서 필요 없으므로 제거한다"는 의도를 명확히 전달할 수 있습니다. 이는 코드의 가독성과 유지보수성을 높일수 있습니다.

DoublesMatch byeMatch = new DoublesMatch(league, byeTeam, null, 1);
Copy link
Collaborator

Choose a reason for hiding this comment

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

null을 여기 전달하는 이유가 있을까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

부전승 매치의 participant2 는 Null 이어야 합니다

Copy link
Collaborator

Choose a reason for hiding this comment

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

LeagueParticipant (entity) 내부에 아래와 같이 부전승을 위한 비어 있는 참여자를 return 하는 정적 팩토리 메서드를 추가하면 더 좋을 것 같습니다! 지금으로서는 "부전승 매치의 participant2 는 Null 이어야 합니다"라는 맥락을 알기가 어려운데, 아래와 같이 변경될 수 있을 것 같아요.

본문의 코드

DoublesMatch match = new DoublesMatch(league, byeTeam, LeagueParticipant.emptyParticipantForByeMatch(), roundNumber);

LeagueParticipant.java (엔티티 코드)

public static LeagueParticipant emptyParticipantForByeMatch() {
		return null;
}

byeMatch.byeMatch();
doublesMatchStore.store(byeMatch);
matches.add(byeMatch);
}
return matches;
}

private List<DoublesMatch> createRoundMatches(League league, List<DoublesMatch> previousMatches, int roundNumber) {
private List<DoublesMatch> createMatchesRound(League league, List<DoublesMatch> previousRoundMatches,
int roundNumber) {
List<DoublesMatch> currentRoundMatches = new ArrayList<>();
for (int i = 0; i < previousMatches.size(); i += TEAMS_PER_MATCH) {

currentRoundMatches.addAll(createRegularMatchesForRound(league, previousRoundMatches, roundNumber));

if (isDoubleMatchOddSize(previousRoundMatches)) {
currentRoundMatches.add(createByeRoundMatch(league, previousRoundMatches, roundNumber));
}

return currentRoundMatches;
}

private List<DoublesMatch> createRegularMatchesForRound(League league, List<DoublesMatch> previousRoundMatches,
int roundNumber) {
List<DoublesMatch> regularRoundMatches = new ArrayList<>();

for (int i = 0; i < previousRoundMatches.size() - 1; i += TEAMS_PER_MATCH) {
DoublesMatch match = new DoublesMatch(league, null, null, roundNumber);
makeSetsInMatch(match);
doublesMatchStore.store(match);
currentRoundMatches.add(match);
regularRoundMatches.add(match);
}
return currentRoundMatches;

return regularRoundMatches;
}

private DoublesMatch createByeRoundMatch(League league, List<DoublesMatch> previousRoundMatches,
int roundNumber) {
DoublesMatch byeMatch = previousRoundMatches.get(previousRoundMatches.size() - 1);
Team winner = determineWinner(byeMatch);

DoublesMatch nextByeMatch = new DoublesMatch(league, winner, null, roundNumber);
nextByeMatch.byeMatch();
doublesMatchStore.store(nextByeMatch);

return nextByeMatch;
}

private List<DoublesMatch> createSubsequentRoundsMatches(League league, int totalRounds) {
List<DoublesMatch> matches = new ArrayList<>();
List<DoublesMatch> previousMatches = doublesMatchReader.findMatchesByLeagueAndRound(league.getLeagueId(), 1);

for (int roundNumber = 2; roundNumber <= totalRounds; roundNumber++) {
List<DoublesMatch> currentRoundMatches = createRoundMatches(league, previousMatches, roundNumber);
List<DoublesMatch> currentRoundMatches = createMatchesRound(league, previousMatches, roundNumber);
matches.addAll(currentRoundMatches);
previousMatches = currentRoundMatches;
}
Expand All @@ -200,9 +273,9 @@ private void makeSetsInMatch(DoublesMatch doublesMatch) {
doublesMatchStore.store(doublesMatch);
}

private void updateSetScore(DoublesMatch doublesMatch, int setIndex,
private void updateSetScore(DoublesMatch doublesMatch, int setNumber,
MatchCommand.UpdateSetScore updateSetScoreCommand) {
DoublesSet set = doublesMatch.getDoublesSet(setIndex);
DoublesSet set = doublesMatch.getDoublesSet(setNumber);
set.endSetScore(updateSetScoreCommand.getScore1(), updateSetScoreCommand.getScore2());

if (updateSetScoreCommand.getScore1() > updateSetScoreCommand.getScore2()) {
Expand Down Expand Up @@ -231,22 +304,36 @@ private void updateNextRoundMatch(DoublesMatch doublesMatch) {
Math.toIntExact(startMatch.getId()));

DoublesMatch nextRoundMatch = doublesMatchReader.getDoublesMatch((long)nextRoundMatchId);
if (nextRoundMatch != null) {
assignWinnerToNextRoundMatch(nextRoundMatch, winner);

if (nextRoundMatch == null) {
return;
}

if (doublesMatch.getMatchStatus() == MatchStatus.BYE) {
nextRoundMatch.defineTeam1(winner);
doublesMatchStore.store(nextRoundMatch);
return;
}

assignWinnerToNextRoundMatch(nextRoundMatch, winner);
doublesMatchStore.store(nextRoundMatch);
}

private void assignWinnerToNextRoundMatch(DoublesMatch nextRoundMatch, Team winner) {
if (nextRoundMatch.getTeam1() == null) {
nextRoundMatch.defineTeam1(winner);
} else {
return;
}
if (nextRoundMatch.getTeam2() == null) {
nextRoundMatch.defineTeam2(winner);
}

}

private Team determineWinner(DoublesMatch match) {
if (match.getMatchStatus() == MatchStatus.BYE) {
return match.getTeam1();
}
if (match.getTeam1MatchResult() == MatchResult.WIN) {
return match.getTeam1();
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

요거

if(match.getMatchStatus() == MatchStatus.BYE || match.getMatchStatus() == MatchResult.WIN) {
   return match.getTeam1();
}

로 갈 수도 있지 않아요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

그렇네요!

Expand Down
Loading