리그 내 팀별 랭킹 기능을 만들었다.
아직 걸리는 점이 좀 있어 좀 다듬어야 할 것 같다.
현재 코드는 다음과 같이 구현되어 있다.
@Component
@RequiredArgsConstructor
public class LeagueRankingScheduler {
private final LeagueService leagueService;
@Scheduled(cron = "0 0 0 * * ?")
public void updateLeagueRankings() {
leagueService.updateRankings();
}
}
LeagueRankingScheduler에서 자정에 updateLeagueRankings이 실행된다.
// LeagueService.java
@Transactional
public void updateRankings() {
List<League> activeLeagues = leagueJpaRepository.findAllBySeasonAndEndAtAfterAndIsDeletedFalse(
LocalDate.now().getYear(), LocalDate.now().minusDays(1)
);
for (League league : activeLeagues) {
List<LeagueGame> yesterdayGames = resultService.findGamesByLeagueAndDate(
league.getLeagueId(), LocalDate.now().minusDays(1)
);
for (LeagueGame game : yesterdayGames) {
updateTeamStats(game);
}
calculateAndSaveRankings(league);
}
}updateRankings 내에서 현재 진행중인 리그를 찾고, 각 리그마다 어제 날짜로 기록된 경기 결과를 조회한다.
각 경기 결과를 팀 점수에 반영(updateTeamStats)하여 랭킹을 산정(calculateAndSaveRankings)한다.
private void updateTeamStats(LeagueGame lg) {
League league = findById(lg.getLeague().getLeagueId());
Game game = lg.getGame();
LeagueTeam homeTeam = findLeagueTeamByTeamIdAndLeagueId(game.getHomeTeamId(),
league.getLeagueId()
);
LeagueTeam awayTeam = findLeagueTeamByTeamIdAndLeagueId(game.getAwayTeamId(),
league.getLeagueId()
);
int homeScore = lg.getHomeScore();
int awayScore = lg.getAwayScore();
int winPoint = 3;
int defeatPoint = 0;
if (homeScore > awayScore) {
homeTeam.updateStats(winPoint, homeScore, awayScore, true, false, false);
awayTeam.updateStats(defeatPoint, awayScore, homeScore, false, true, false);
} else if (homeScore < awayScore) {
homeTeam.updateStats(defeatPoint, homeScore, awayScore, false, true, false);
awayTeam.updateStats(winPoint, awayScore, homeScore, true, false, false);
} else {
int drawPoint = 1;
homeTeam.updateStats(drawPoint, homeScore, awayScore, false, false, true);
awayTeam.updateStats(drawPoint, awayScore, homeScore, false, false, true);
}
}경기 결과를 팀 점수에 반영하는 메서드이다. 경기 결과에 승팀이 따로 기록되지는 않기 때문에, 두 팀의 점수를 비교한다.
승리 시 +3, 무승부 +1, 패배 +0 으로 계산하고 있다.
public void updateStats(
int points, int goalsFor, int goalsAgainst,
boolean isWin, boolean isLoss, boolean isDraw
) {
this.teamScore += points;
this.totalScore += goalsFor;
this.totalLoss += goalsAgainst;
if (isWin) {
this.winCnt++;
}
if (isLoss) {
this.defeatCnt++;
}
if (isDraw) {
this.drawCnt++;
}
}LeagueTeam 엔티티 내의 updateStats 메서드이다. 경기 결과에 따른 점수와 경기 내 득점, 경기 내 실점과 결과를 받아서 계산한다. teamScore는 승리/무승부/패배시의 점수, totalScore는 경기 내 득점, totalLoss는 경기 내 실점을 반영한다.
private void calculateAndSaveRankings(League league) {
List<LeagueTeam> leagueTeams = leagueTeamJpaRepository
.findAllByLeagueIdAndIsDeletedFalse(league.getLeagueId());
for (LeagueTeam team : leagueTeams) {
int totalGames = team.getWinCnt() + team.getDefeatCnt() + team.getDrawCnt();
TeamScore teamScore = teamScoreJpaRepository.findByTeamIdAndIsDeletedFalse(team.getTeamId())
.orElseGet(() -> TeamScore.builder()
.teamId(team.getTeamId())
.currentWinRate(BigDecimal.ZERO)
.totalWinRate(BigDecimal.ZERO)
.currentRank(null)
.build()
);
if (totalGames > 0) {
BigDecimal winRate = BigDecimal.valueOf((double) team.getWinCnt() / totalGames * 100);
BigDecimal totalWinRate = teamScore.getTotalWinRate()
.add(BigDecimal.valueOf((double) team.getWinCnt() / totalGames * 100));
teamScore = teamScore.toBuilder()
.currentWinRate(winRate)
.totalWinRate(totalWinRate)
.build();
}
teamScoreJpaRepository.save(teamScore);
}
leagueTeams.sort((a, b) -> {
if (b.getTeamScore() != a.getTeamScore()) {
return b.getTeamScore() - a.getTeamScore();
}
int goalDifferenceA = a.getTotalScore() - a.getTotalLoss();
int goalDifferenceB = b.getTotalScore() - b.getTotalLoss();
if (goalDifferenceB != goalDifferenceA) {
return goalDifferenceB - goalDifferenceA;
}
return b.getTotalScore() - a.getTotalScore();
});
int rank = 1;
for (LeagueTeam team : leagueTeams) {
TeamScore teamScore = findByTeamId(team.getTeamId());
teamScore = teamScore.toBuilder()
.currentRank(rank++)
.build();
teamScoreJpaRepository.save(teamScore);
}
}calculateAndSaveRankings에서는 어제자 경기 결과가 반영된 LeagueTeam을 가지고 TeamScore 엔티티를 업데이트한다.
각 팀의 TeamScore를 찾고, 없으면 생성해주고 만약 총 경기수가 0 이상이라면 승률을 계산하여 반영한다.
winRate는 현재 리그 승률이고, totalWinRate는 전체 경기의 승률이 누적된다.
모든 계산 후 leagueTeams에 대한 랭킹을 순서대로 매긴다.
가장 먼저 teamScore로 판단 후, 동점이라면 두 팀의 총 득실점 차, 그래도 동점이라면 그냥 총점으로 비교한다.
기능은 얼추 구현한 것 같지만 마음에 들진 않는다. 개선이 필요하다.
- 트랜젝션 분리
현 코드에서는 updateRankings 메서드 내에서 많은 작업이 수행되고 있다.
작업 중 실패 시 전체 롤백이 된다는 것이다.. 이 작업 단위를 쪼갤 필요가 있을 것 같다.
현재 비즈니스 로직에서 가장 좋은 구분 단위는 리그인 것 같다. 리그 단위로 쪼개준다!
@Transactional
public void updateRankings() {
List<League> activeLeagues = leagueJpaRepository.findAllBySeasonAndEndAtAfterAndIsDeletedFalse(
LocalDate.now().getYear(), LocalDate.now().minusDays(1)
);
for (League league : activeLeagues) {
processLeagueUpdates(league);
}
}
@Transactional
public void processLeagueUpdates(League league) {
List<LeagueGame> yesterdayGames = resultService.findGamesByLeagueAndDate(
league.getLeagueId(), LocalDate.now().minusDays(1));
for (LeagueGame game : yesterdayGames) {
updateTeamStats(game);
}
calculateAndSaveRankings(league);
}Call transactional methods via an injected dependency instead of directly via 'this'.Spring에서는 @Transactional이 프록시를 통해 동작한다.
동일 클래스 내에서 this를 사용해 직접 호출 시 트랜젝션 프록시가 제대로 적용되지 않을 수 있다.
-> 외부 호출로 바꾸거나 한군데에서 제거해버리면 된다.
updateRankings에서 @Transactional을 제거할 시 예상되는 문제점:
-> 일부 리그 업데이트 실패 시 전체 일관성이 유지되지 않을 수 있다.
각 리그별 랭킹이 중요하지 전체 리그의 순위같은 건 필요치 않기 때문에 괜찮다.
updateRankings에서 @Transactional을 제거해줬다.
@Transactional self-invocation (in effect, a method within the target object calling another method of the target object) does not lead to an actual transaction at runtime
"processLeagueUpdates's" @Transactional requirement is incompatible with the one for this method.
Call transactional methods via an injected dependency instead of directly via 'this'.Spring은.. 프록시 패턴을 기반으로 작동한다...
updateRankings에서 트랜젝션이 적용되지 않았고, updateRankings에서 processLeagueUpdates를 호출 시 self-invocation으로 간주된다.
트랜젝션은 Spring이 생성한 프록시 객체를 통해야만 작동되기 때문에 동일 클래스 내에서 호출 시 트랜젝션이 시작될 수 없다.
그냥 분리해주는 게 맞는 것 같다.
코드도 길었어서 LeagueUpdateService를 만들어서 processLeagueUpdates 관련 메서드를 옮겨줬다.
원래는 LeagueRankingScheduler에서 매일 자정마다 scheduled로 랭킹 업데이트 메서드(LeagueService-updateRankings)를 호출해서 업데이트를 진행했는데, 이렇게 메서드가 타 서비스로 분리되다보니 굳이 밖에서 호출 할 필요도 없어졌다.
LeagueRankingScheduler를 제거하고, updateRankings에 scheduled 처리를 해줬다.
깔끔!
- 전체적으로 정리한 최종 코드
// leagueService 에서 스케쥴링
@Scheduled(cron = "0 0 0 * * ?")
public void updateRankings() {
List<League> activeLeagues = leagueJpaRepository.findAllBySeasonAndEndAtAfterAndIsDeletedFalse(
LocalDate.now().getYear(), LocalDate.now().minusDays(1)
);
for (League league : activeLeagues) {
leagueUpdateService.processLeagueUpdates(league);
}
}
---
```java
// leagueUpdateService 에서 업데이트 수행
@Transactional
public void processLeagueUpdates(League league) {
List<LeagueGame> yesterdayGames = findGamesByLeagueAndDate(
league.getLeagueId(), LocalDate.now().minusDays(1));
for (LeagueGame game : yesterdayGames) {
updateTeamStats(game);
}
calculateAndSaveRankings(league);
}
private void updateTeamStats(LeagueGame lg) {
League league = lg.getLeague();
Game game = lg.getGame();
LeagueTeam homeTeam = findLeagueTeamByTeamIdAndLeagueId(game.getHomeTeamId(),
league.getLeagueId()
);
LeagueTeam awayTeam = findLeagueTeamByTeamIdAndLeagueId(game.getAwayTeamId(),
league.getLeagueId()
);
int homeScore = lg.getHomeScore();
int awayScore = lg.getAwayScore();
int winPoint = 3;
int defeatPoint = 0;
if (homeScore > awayScore) {
homeTeam.updateStats(winPoint, homeScore, awayScore, true, false, false);
awayTeam.updateStats(defeatPoint, awayScore, homeScore, false, true, false);
} else if (homeScore < awayScore) {
homeTeam.updateStats(defeatPoint, homeScore, awayScore, false, true, false);
awayTeam.updateStats(winPoint, awayScore, homeScore, true, false, false);
} else {
int drawPoint = 1;
homeTeam.updateStats(drawPoint, homeScore, awayScore, false, false, true);
awayTeam.updateStats(drawPoint, awayScore, homeScore, false, false, true);
}
}
private void calculateAndSaveRankings(League league) {
List<LeagueTeam> leagueTeams = leagueTeamJpaRepository
.findAllByLeagueIdAndIsDeletedFalse(league.getLeagueId());
List<TeamScore> teamScores = new ArrayList<>();
for (LeagueTeam team : leagueTeams) {
int totalGames = team.getWinCnt() + team.getDefeatCnt() + team.getDrawCnt();
TeamScore teamScore = teamScoreJpaRepository.findByTeamIdAndIsDeletedFalse(team.getTeamId())
.orElseGet(() -> TeamScore.builder()
.teamId(team.getTeamId())
.currentWinRate(BigDecimal.ZERO)
.totalWinRate(BigDecimal.ZERO)
.currentRank(null)
.build()
);
if (totalGames > 0) {
BigDecimal winRate = BigDecimal.valueOf((double) team.getWinCnt() / totalGames * 100);
BigDecimal totalWinRate = teamScore.getTotalWinRate()
.add(BigDecimal.valueOf((double) team.getWinCnt() / totalGames * 100));
teamScore = teamScore.toBuilder()
.currentWinRate(winRate)
.totalWinRate(totalWinRate)
.build();
}
teamScores.add(teamScore);
}
Comparator<LeagueTeam> rankingComparator = Comparator
.comparingInt(LeagueTeam::getTeamScore).reversed()
.thenComparingInt(team -> team.getTotalScore() - team.getTotalLoss()).reversed()
.thenComparingInt(LeagueTeam::getTotalScore).reversed();
leagueTeams.sort(rankingComparator);
for (int i = 0; i < leagueTeams.size(); i++) {
LeagueTeam team = leagueTeams.get(i);
TeamScore teamScore = findByTeamId(team.getTeamId());
teamScore = teamScore.toBuilder()
.currentRank(i + 1)
.build();
teamScores.set(i, teamScore);
}
teamScoreJpaRepository.saveAll(teamScores);
leagueTeamJpaRepository.saveAll(leagueTeams);
}25.01.20
제출한 문서를 바탕으로 튜터님 피드백이 왔다.
isWin, isLose, isDraw 같은 flag 변수들은 코드를 자세하게 알고 있지 않는 이상 실수를 할 확률이 높기 때문에 지양하시는 것이 좋습니다.
그래서, updateState 함수로 모든 기능을 커버하지 말고, 별도의 함수의 네이밍으로 써 표현하는 것이 더욱 좋아 보이네요!
updateWinStatus(winPoint, homeScore, awayScore)
updateLoseStatus(…)
updateDrawStatus(…)사실 고민했던 부분이었다. 그제 클린 코드 특강에서 플래그 함수 사용 지양에 대한 설명을 들었었기 때문에..
알면서도 왜 그랬냐고 묻는다면, 음.
크게 경각심을 가지지 못했던 것 같다.
플래그 함수를 남발하면 쓰는 입장에서는 간단할지 몰라도, 다른 사람이 보았을 때 헷갈리거나 실수하는 일이 생길 확률이 높다.
튜터님도 엔티티까지 가서 updatestats 메서드를 확인하고 알았다고 하시니, 좋은 방법은 아닌 것이다.
솔직히 코드 다 짜고 지금 보니 나도 헷갈린다.
