Skip to content

매치 전적 구현#39

Merged
hojooo merged 7 commits intodevelopfrom
feat/#30-club-statistics
Jun 30, 2025
Merged

매치 전적 구현#39
hojooo merged 7 commits intodevelopfrom
feat/#30-club-statistics

Conversation

@hojooo
Copy link
Collaborator

@hojooo hojooo commented May 29, 2025

매치 전적 구현

📌 변경 사항 (What’s changed?)

  • statistic 엔티티 추가
  • 생성, 업데이트 로직 구현

⚙️ 변경 이유 (Why?)

  • 매치 전적을 관리할 도메인 구현

✅ 테스트 방법 (How to test?)

  • 아직 테스트는 못해봤습니다 ㅜㅜ 추후 테스트 하도록 하겠습니다

🤔 기타 참고 사항

  • 테스트 미흡.. 추후 리팩터링하고 api확인할 때 한 번에 확인해보겠습니다!
  • 편하게 피드백 주세요!

@hojooo hojooo self-assigned this May 29, 2025
@hojooo hojooo requested review from 5nam and y3binchoi as code owners May 29, 2025 14:51
@hojooo hojooo added the ✨ feature New feature or request label May 29, 2025
Copy link
Collaborator

@5nam 5nam left a comment

Choose a reason for hiding this comment

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

전적 로직이 복잡한데, 구현하시느라 고생 많으셨습니다! :)

import lombok.AllArgsConstructor;
import lombok.Data;

public class StatisticDTO {
Copy link
Collaborator

Choose a reason for hiding this comment

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

StatisticDTO에 Request, Response 모아서 구현하신 이유가 궁금합니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

DTO들이 많아지면 관리하기가 힘들어진다고 생각이 들어서 이너 클래스로 관련된 DTO들을 모아서 구현했습니다!

int homeRankLevel = homeStatistic.getTier().getLevel();
int awayRankLevel = awayStatistic.getTier().getLevel();

StatisticDTO.mvpDTO homeMVPResult = statisticRepository.findTopScorerByClubAndSportsType(match.getHomeClub(), currentSeason, SportsType.OVERALL).get(0);
Copy link
Collaborator

Choose a reason for hiding this comment

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

전적 데이터가 있는지 확인하는 orElseThrow 등의 로직이 있으면 좋을 것 같습니다!(.get(0) 이 NullPointException 가능성이 있어 보입니다)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

엇 그걸 생각하지 못했네요! 피드백 감사합니다!!

Copy link
Collaborator

@y3binchoi y3binchoi left a comment

Choose a reason for hiding this comment

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

매치 전적 부분이 취합할게 많아 고민이 많으셨을텐데 수고하셨습니다! 확실히 이 부분은 테스트를 넣어서 반드시 흐름을 확인해보고 배포해야 할 것 같아요!

public class StatisticDTO {
@Data
@AllArgsConstructor
public static class StatisticRequest {
Copy link
Collaborator

Choose a reason for hiding this comment

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

기존 코드 규칙에 따라 RequestBody, ResponseBody가 되는 DTO를 모두 XxxInput, XxxOutput으로 통일하는 게 좋을 것 같아요!

public StatisticControllerAdvice(ResponseCode responseCode) {
super(responseCode);
}
// @ExceptionHandler
Copy link
Collaborator

Choose a reason for hiding this comment

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

거의 확실하게 안쓰이는 코드라면 삭제해도 좋을 것 같습니다! 버전 관리 기록에 남아있을테니까요 :)

@hojooo hojooo merged commit 897068a into develop Jun 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

✨ feature New feature or request

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants