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

I-migi · 2024-12-05T06:09:49Z

PR에 대한 설명 🔍

토너먼트 복식 부전승 로직구현했습니다.

PR에서 중점적으로 확인되어야 하는 사항

토너먼트 싱글과 복식에 중복코드가 많습니다. 다음 이슈에서 리팩토링 시도해 볼 예정입니다

soeunnPark

확인해주세요!

soeunnPark · 2024-12-05T06:18:29Z

...tructure/src/main/java/org/badminton/infrastructure/match/reader/DoublesMatchReaderImpl.java

@@ -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);


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

soeunnPark · 2024-12-05T06:21:15Z

...rc/main/java/org/badminton/infrastructure/match/strategy/TournamentDoublesMatchStrategy.java

@@ -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)


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

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

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

soeunnPark · 2024-12-05T06:26:47Z

...rc/main/java/org/badminton/infrastructure/match/strategy/TournamentDoublesMatchStrategy.java

@@ -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());


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

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

soeunnPark · 2024-12-05T06:31:32Z

...rc/main/java/org/badminton/infrastructure/match/strategy/TournamentDoublesMatchStrategy.java

+		if (isParticipantOddSize(participants)) {
+			Team byeTeam = new Team(participants.remove(participants.size() - 1),
+				participants.get(participants.size() - 2));
+			DoublesMatch byeMatch = new DoublesMatch(league, byeTeam, null, 1);


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

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

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

본문의 코드

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

LeagueParticipant.java (엔티티 코드)

public static LeagueParticipant emptyParticipantForByeMatch() { return null; }

soeunnPark · 2024-12-05T06:31:57Z

...rc/main/java/org/badminton/infrastructure/match/strategy/TournamentDoublesMatchStrategy.java

+		if (isParticipantOddSize(participants)) {
+			Team byeTeam = new Team(participants.remove(participants.size() - 1),
+				participants.get(participants.size() - 2));


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

remove를 하는 목적은 뭘까요?

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

soeunnPark · 2024-12-05T06:35:24Z

...rc/main/java/org/badminton/infrastructure/match/strategy/TournamentDoublesMatchStrategy.java

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


요거

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

로 갈 수도 있지 않아요?

그렇네요!

soeunnPark

수정사항 확인했습니다! 수고하셨습니다~!

soeunnPark · 2024-12-05T08:10:23Z

domain/src/main/java/org/badminton/domain/common/error/ErrorCode.java

@@ -104,7 +104,7 @@ public enum ErrorCode {
 	CLUB_OWNER_CANT_WITHDRAW(412, "동호회원이 2명 이상인 동호회 회장은 동호회를 탈퇴할 수 없습니다."),
 	NOT_LEAGUE_OWNER(412, "경기를 만든 사용자만 수정 및 삭제를 할 수 있습니다"),
 	CLUB_MEMBER_EXPEL_EXCEPTION(412, "해당 동호회에서 제제를 받아 가입 신청을 할 수 없습니다."),
-	BYE_MATCH(412, "해당 매치는 부전승 매치 입니다"),
+	BYE_MATCH_ACTION_NOT_ALLOWED(412, "해당 매치는 부전승 매치이기 때문에 점수를 수정할 수 없습니다"),


ACTION이 조금 모호한데.. SCORE_UPDATE_NOT_ALLOWED_IN_BYE_MATCH는 어떨까요? 😀

soeunnPark · 2024-12-05T08:11:13Z

...tructure/src/main/java/org/badminton/infrastructure/match/reader/DoublesMatchReaderImpl.java

@@ -64,12 +65,13 @@ public boolean allMatchesFinishedForLeague(Long leagueId) {

 	@Override
 	public boolean allMatchesNotStartedForLeague(Long leagueId) {
-		return doublesMatchRepository.allMatchesNotStartedForLeague(leagueId);
+		List<MatchStatus> UnPlayedMatchStatuses = Arrays.asList(MatchStatus.NOT_STARTED, MatchStatus.BYE);
+		return doublesMatchRepository.allMatchesNotStartedForLeague(leagueId, UnPlayedMatchStatuses);


변수명은 소문자로 적어야 합니다.

soeunnPark

리뷰 추가했습니다.

soeunnPark · 2024-12-05T08:13:10Z

...tructure/src/main/java/org/badminton/infrastructure/match/reader/DoublesMatchReaderImpl.java

 	}

 	@Override
 	public boolean allRoundMatchesDone(Long leagueId, int roundNumber) {
-		List<MatchStatus> statuses = List.of(MatchStatus.FINISHED, MatchStatus.BYE);
-		return doublesMatchRepository.areAllMatchesFinishedOrBye(leagueId, roundNumber, statuses);
+		List<MatchStatus> CompletedOrBypassedStatuses = List.of(MatchStatus.FINISHED, MatchStatus.BYE);


변수명은 소문자로 적어야 합니다 22

soeunnPark · 2024-12-05T08:14:36Z

...rc/main/java/org/badminton/infrastructure/match/strategy/TournamentDoublesMatchStrategy.java

@@ -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)


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

soeunnPark · 2024-12-05T08:15:09Z

...rc/main/java/org/badminton/infrastructure/match/strategy/TournamentDoublesMatchStrategy.java

+		updateNextRoundMatch(doublesMatch);
+		boolean allRoundMatchesDone = doublesMatchReader.allRoundMatchesDone(doublesMatch.getLeague().getLeagueId(),
+			doublesMatch.getRoundNumber());
+		boolean currentRoundInTotalRound =


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

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

soeunnPark · 2024-12-05T08:16:54Z

...rc/main/java/org/badminton/infrastructure/match/strategy/TournamentDoublesMatchStrategy.java

@@ -69,6 +80,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)
+			.forEach(this::updateNextRoundMatch);


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

…yematch

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

565a154

I-migi added the feat✨ 기능 구현 label Dec 5, 2024

I-migi requested a review from a team December 5, 2024 06:09

I-migi self-assigned this Dec 5, 2024

I-migi linked an issue Dec 5, 2024 that may be closed by this pull request

[feat] Doubles Tournament League plus ByeMatch #521

Closed

1 task

soeunnPark reviewed Dec 5, 2024

View reviewed changes

리뷰 반영

0b61057

soeunnPark approved these changes Dec 5, 2024

View reviewed changes

soeunnPark reviewed Dec 5, 2024

View reviewed changes

I-migi and others added 2 commits December 5, 2024 17:24

Merge branch 'develop' into 521-feat-doubles-tournament-league-plus-b…

2a7593f

…yematch

토너먼트 싱글 MatchStatus 파라미터 추가

90fe8ed

I-migi merged commit 899ac6e into develop Dec 5, 2024
1 check passed

I-migi deleted the 521-feat-doubles-tournament-league-plus-byematch branch December 5, 2024 08:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

I-migi commented Dec 5, 2024

soeunnPark left a comment

soeunnPark Dec 5, 2024

soeunnPark Dec 5, 2024

I-migi Dec 5, 2024

soeunnPark Dec 5, 2024

soeunnPark Dec 5, 2024

soeunnPark Dec 5, 2024

soeunnPark Dec 5, 2024

I-migi Dec 5, 2024

soeunnPark Dec 5, 2024

soeunnPark Dec 5, 2024

soeunnPark Dec 5, 2024

I-migi Dec 5, 2024

soeunnPark Dec 5, 2024

I-migi Dec 5, 2024

soeunnPark left a comment

soeunnPark Dec 5, 2024

soeunnPark Dec 5, 2024

soeunnPark left a comment

soeunnPark Dec 5, 2024

soeunnPark Dec 5, 2024

soeunnPark Dec 5, 2024

I-migi Dec 5, 2024

soeunnPark Dec 5, 2024

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

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

Conversation

I-migi commented Dec 5, 2024

PR에 대한 설명 🔍

PR에서 중점적으로 확인되어야 하는 사항

soeunnPark left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

본문의 코드

LeagueParticipant.java (엔티티 코드)

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

soeunnPark left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

soeunnPark left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment