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

[refactor] #221 - 메인페이지에서 캐러셀 응답 시 캐러셀 번호로 정렬해서 주도록 변경 및 메인 페이지 조회 코드 최적화 #222

Merged
merged 12 commits into from
Oct 2, 2024

Conversation

hoonyworld
Copy link
Member

@hoonyworld hoonyworld commented Oct 1, 2024

Related issue 🛠

Work Description ✏️

  • 메인페이지에서 캐러셀 응답 시 캐러셀 번호로 정렬해서 주도록 변경하였습니다.
  • 또한 기존에 불필요하게 길었던 코드를 최적화 하였습니다.
  • 또한 각 도메인에 해당하는 서비스 로직을 호출할 수 있도록 리팩토링 하였습니다. (단, 현재 모든 도메인에 adapter&port 패턴이 적용된 것이아니기에 usecase와 service가 같이 놓여져 있는 경우가 있습니다.)

Trouble Shooting ⚽️

Related ScreenShot 📷

메인페이지에서 캐러셀 응답 시 캐러셀 번호로 정렬해서 Return하는지 확인

image image
  • DB에 저장된 id 순서로 return 하는 것이 아닌, 캐러셀번호 순서대로 정렬해서 return 하는 것을 확인했습니다.

메인페이지 조회 코드 최적화 후 정상 작동하는 지 확인

image image image image image
  • 순서대로 태그 x, BAND, PLAY, DANCE, ETC의 필터링이 잘 작동하는지 확인했습니다.

dueDate 관련 음수는 내림차순 정렬, 양수는 오름차순 정렬 확인

image
  • 양수는 오름차순 음수는 내림차순으로 정렬되는 것을 확인했습니다.

Uncompleted Tasks 😅

To Reviewers 📢

Comment on lines 47 to 50
.sorted(Comparator.comparingInt(HomePerformanceDetail::dueDate)
.reversed()
.thenComparingInt(detail -> detail.dueDate() >= 0 ? -1 : 1))
.toList();
Copy link
Collaborator

@hyerinhwang-sailin hyerinhwang-sailin Oct 2, 2024

Choose a reason for hiding this comment

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

홈화면에서 공연 정렬 기준은 duedate가 양수일 경우 오름차순, 이후 음수는 내림차순입니다!
예를 들어 0, 2, 4, 15, -1, -4, -10 이런식으로요. 지금 코드는 양수인 경우에도 내림차순으로 정렬하고 있는 것 같습니다ㅠㅠ

List<HomePerformanceDetail> sortedPerformances = performances.stream()
    .map(performance -> {
        int minDueDate = scheduleService.getMinDueDateForPerformance(performance.getId());
        return HomePerformanceDetail.of(performance, minDueDate);
    })
    .sorted(Comparator.<HomePerformanceDetail>comparingInt(detail -> detail.dueDate() >= 0 ? 0 : 1)  // 양수는 0, 음수는 1로 구분
        .thenComparingInt(detail -> detail.dueDate() >= 0 ? detail.dueDate() : detail.dueDate()))  // 양수는 오름차순, 음수는 내림차순
    .toList();

이렇게 바꾸면 어떨까요?

로컬 테스트하실 때 과거 공연과 미래 공연 섞어서 홈화면 잘 정렬되는지 확인해주시면 좋을 것 같습니다!

Copy link
Member Author

Choose a reason for hiding this comment

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

제가 놓쳤던 부분이였네요!!

그런데, 지금 보내주신 코드도 음수 정렬의 경우 오름차순 정렬을 해주고 있어서 다음과 같이 반영하겠습니다!

List<HomePerformanceDetail> sortedPerformances = performances.stream()
	.map(performance -> {
		int minDueDate = scheduleService.getMinDueDateForPerformance(performance.getId());
		return HomePerformanceDetail.of(performance, minDueDate);
	})
	.sorted(Comparator.<HomePerformanceDetail>comparingInt(detail -> detail.dueDate() < 0 ? 1 : 0)
		.thenComparingInt(detail -> Math.abs(detail.dueDate())))
	.toList();

절댓값을 비교해서 정렬을 해주는 로직을 도입하여 양수는 그대로 오름차순, 음수는 절댓값을 씌워서 정렬하면 내림차순으로 정렬되는 점을 이용했습니다!

Copy link
Member Author

Choose a reason for hiding this comment

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

pr 내용에도 추가했습니다!

Copy link
Collaborator

Choose a reason for hiding this comment

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

저도 처음엔 절댓값을 씌우는 방식을 고려했는데 더 알아본 결과 comparator가 원래 양수는 오름차순, 음수는 내림차순으로 정렬하더라구요!
https://bono039.tistory.com/847
링크 참고해주세요!

Copy link
Member Author

@hoonyworld hoonyworld Oct 2, 2024

Choose a reason for hiding this comment

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

comparator는 양수는 오름차순, 음수는 내림차순으로 정렬하는 것이 아닌 양수든 음수든 결국 같이 오름차순 or 내림차순으로 정렬하는 로직입니다.

Comparator.compare(a, b)에서:

  • a - b가 양수일 경우: b가 a보다 작기 때문에 b가 앞에 오고 a는 뒤로 갑니다.
  • a - b가 음수일 경우: a가 b보다 작기 때문에 a는 앞에 남고 b가 뒤로 갑니다.
  • b - a가 양수일 경우: b가 a보다 크기 때문에 b가 앞에 오고 a는 뒤로 갑니다.
  • b - a가 음수일 경우: a가 b보다 크기 때문에 a가 앞에 오고 b는 뒤로 갑니다.

예를 들어, a=-2, b=-4라 가정할 때 a-b 로직을 사용하면 -2 - (-4) = +2 이므로 양수기 때문에 -4가 앞에오고 -2는 뒤로 가게 됩니다.
이때는 오름차순 정렬이 되게됩니다.

반면, a=-2, b=-4라 가정할 때 b-a 로직을 사용하면 -4 - (-2) = -2 이므로 음수기 때문에 -2가 앞에오고 -4는 뒤로 가게 됩니다.
이때는 내림차순 졍렬이 되게됩니다.

따라서 a-b냐 b-a냐에 따라 양수든 음수든 오름차순으로 혹은 내림차순으로 정렬이 되게됩니다!

Copy link
Collaborator

Choose a reason for hiding this comment

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

엇 그렇군요! 제가 잘못 알고 있었나보네요 감사합니다~

Copy link
Collaborator

@hyerinhwang-sailin hyerinhwang-sailin left a comment

Choose a reason for hiding this comment

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

캐러셀 넘버 순 정렬, 코드 간소화 다 좋습니다!
다만 홈화면 공연 정렬 기준에서 수정이 필요한 부분이 있어보여 리뷰 남겼으니 확인 부탁드립니다~

Copy link
Collaborator

@hyerinhwang-sailin hyerinhwang-sailin left a comment

Choose a reason for hiding this comment

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

lgtm~

@hoonyworld hoonyworld merged commit 2dbb3a4 into develop Oct 2, 2024
1 check passed
@hoonyworld hoonyworld deleted the refactor/#221 branch October 2, 2024 12:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[refactor] 메인 페이지 조회 시, 캐러셀 리스트를 캐러셀 번호 순으로 정렬하여 응답
2 participants