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] 매니저가 관리하는 리그 전체 조회 기능 구현(대회 관리 페이지) #226

Merged
merged 10 commits into from
Sep 23, 2024

Conversation

Zena0128
Copy link
Contributor

🌍 이슈 번호

📝 구현 내용

  • 대회 관리 페이지에서 사용될, 매니저가 관리하는 리그 전체 조회 기능 구현
  • 경기 중 -> 시작 전 -> 종료 순으로 정렬되도록 구현

🍀 확인해야 할 부분

  • 경기 중 -> 시작 전 -> 종료 순 정렬을 확인하기 위해 league-fixture.sql에 데이터 추가했습니당(연관된 테스트 코드들도 함께 잘 작동하도록 수정 완료)
  • 비슷한 결의 DTO들은 하나의 클래스에 묶어서 관리하는 게 좋을지? ex. LeagueResponse어쩌고 레코드들을 다 하나로 묶기?

Copy link
Member

@hodadako hodadako left a comment

Choose a reason for hiding this comment

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

고생하셨습니다. 리뷰 남긴거 확인해주세요~

Comment on lines 115 to 125
Map<String, Integer> orderMap = new HashMap<>();
orderMap.put(LeagueProgress.IN_PROGRESS.getDescription(), 1);
orderMap.put(LeagueProgress.BEFORE_START.getDescription(), 2);
orderMap.put(LeagueProgress.FINISHED.getDescription(), 3);

Comparator<League> comparator = Comparator.comparing(
league -> orderMap.get(LeagueProgress.getProgressDescription(LocalDateTime.now(), league)));

return leagues.stream()
.sorted(comparator)
.map(LeagueResponseToManage::new)
Copy link
Member

Choose a reason for hiding this comment

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

  1. 정렬 순서에 대한 요구사항이 변경된다면 어떻게 될까요? (시작 전, 진행 중, 완료)
  2. Map의 정적 팩토리 메서드를 사용하면 어떤 식으로 코드를 바꿀 수 있을까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

1의 경우 맵을 static으로 구현하지 않아 정렬 순서 변경 시 서비스단, 서비스 테스트단 두 군데에서 변경해야 한다는 의미에서 주신 질문인가요?-?

Copy link
Contributor

@Jin409 Jin409 left a comment

Choose a reason for hiding this comment

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

저도 DTO 가 좀 많은 것 같아서 살펴보니 LeagueDetailResponse 와 id 빼고 전부 일치하네요..!
LeagueDetailResponse 에 id 를 추가하거나 하는 방식으로 DTO 개수를 줄이는 건 어떨까요?

@Zena0128
Copy link
Contributor Author

저도 DTO 가 좀 많은 것 같아서 살펴보니 LeagueDetailResponse 와 id 빼고 전부 일치하네요..! LeagueDetailResponse 에 id 를 추가하거나 하는 방식으로 DTO 개수를 줄이는 건 어떨까요?

  • DTO가 너무 병렬적으로 많긴 해서 리그 관련 정보 나타내는 LeagueResponse 클래스를 만들어서 거기 안에다 관련 dto 레코드들 다 묶어서 올리는 것이 어떨지..!

  • LeagueDetailResponse랑 대회 관리 페이지에서 사용되는 dto랑 공통으로 사용? : 둘이 사용되는 곳이 다르다보니 나중에 또 다시 분리되어야 할 수도 있을 것 같아서(각 페이지에서 리턴하는 필드 추가/삭제될 수 있음) 그냥 지금처럼 따로 사용하는 걸로..

요렇게 승희님과 대면으로 논의했습니당

@Zena0128 Zena0128 merged commit 425f6d2 into main Sep 23, 2024
1 check passed
@Zena0128 Zena0128 deleted the feat/#217-league branch September 23, 2024 05:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants