[BE] 최소 시간을 보장하는 추천 로직 추가 #404

ikjo39 · 2024-10-13T13:53:53Z

작업 내용

최소 시간을 보장하는 로직을 추가하였습니다.

최소 시간 기준

최소 시간의 경우 30분, 60분, 120분과 같이 현재 시간 로직과 동일하게 30분 단위로 기준을 잡았습니다.

최소 시간 보장 로직

최소 시간 보장하는 로직을 어디에 둘지 고민했는데요. 제가 생각한 최소 요구사항은 아래와 같았습니다.

sort 기준을 Duration 크기의 역순으로 걸고 최소 시간보다 작은 것은 필터링한다.
출력할 땐 참가자가 많은 순서대로 정렬 후 시간이 빠른 순(Earliest)으로 정렬한다.

적절한 위치를 고민하다 CandidateSchedule내에 mergeContinuous 안에 위치시켰습니다.
반복문을 필연적으로 돌아야 하는 곳에서 분기 처리 하는게 효율적이라 생각하였어요 :)

특이 사항

추천 로직 API 에 대해 요구사항이 변경되었습니다!
다만, 기존 로직과 충돌을 방지하기 위해 새로 추가된 @RequestParam인 minTime에 기본 값을 설정해두었습니다.

리뷰 요구사항

전체적으로 로직에서 불합리한 부분이 없는지와 테스트가 부족한 부분이 없는지 확인 부탁 드려요~

github-actions · 2024-10-13T13:55:31Z

Test Results

165 tests +130 165 ✅ +130 22s ⏱️ -3s
32 suites + 28 0 💤 ± 0
32 files + 31 0 ❌ ± 0

Results for commit c848d79. ± Comparison against base commit 23c88c0.

♻️ This comment has been updated with latest results.

seokmyungham

다온 고생하셨습니다 😁
받으면 기분이 좋아지는 RC를 드립니다 🚀

seokmyungham · 2024-10-17T06:10:45Z

backend/src/main/java/kr/momo/domain/schedule/recommend/CandidateSchedule.java

+        if (minSize < MINIMUM_MIN_SIZE) {
+            throw new MomoException(ScheduleErrorCode.INVALID_MIN_TIME);
+        }


서비스에서 이 검증을 수행한 다음에 추천 로직을 수행하는게 자연스러울 것 같아요

흠 그러면 controller의 parameter 단에서 validation 라이브러리를 통해 검증하는건 어떨까요?

비즈니스 정책에 해당한다고 생각해서 비즈니스 로직이 수행되는 위치에서 수행되어야 할 것 같아요.

seokmyungham · 2024-10-17T06:12:34Z

backend/src/main/java/kr/momo/domain/schedule/recommend/CandidateSchedule.java

+        if (minSize > subList.size()) {
+            return;
+        }
+        subList.stream()
+                .reduce(CandidateSchedule::merge)
+                .ifPresent(mergedSchedules::add);


Suggested change

if (minSize > subList.size()) {

return;

}

subList.stream()

.reduce(CandidateSchedule::merge)

.ifPresent(mergedSchedules::add);

if (subList.size() >= minSize) {

subList.stream()

.reduce(CandidateSchedule::merge)

.ifPresent(mergedSchedules::add);

}

이건 어떤가요??

Early Return을 하는 것과 가독성을 고민하였는데요.
아래가 더 가독성이 좋아보이네요 :)

seokmyungham · 2024-10-17T06:16:02Z

backend/src/main/java/kr/momo/service/schedule/ScheduleService.java

+                recommendedResult
+        );


다온이 만든 코드는 아니지만 😅
지금 dto 변환 작업을 두 번에 나눠서 하고 있는데 한 번에 하면 더 좋을 것 같아요

이 작업은 리팩토링 이슈로 분리하여 다뤄봐도 좋을 것 같네요

seokmyungham · 2024-10-17T06:33:42Z

backend/src/main/java/kr/momo/controller/schedule/ScheduleControllerDocs.java

@@ -98,9 +98,11 @@ MomoApiResponse<AttendeeScheduleResponse> findMySchedule(
    @ApiSuccessResponse.Ok("추천 일정 조회 성공")
    MomoApiResponse<RecommendedSchedulesResponse> recommendSchedules(
            @PathVariable @Schema(description = "약속 UUID") String uuid,
-            @RequestParam @Schema(description = "추천 기준(이른 시간 순 / 길게 볼 수 있는 순)", example = "earliest")
+            @RequestParam @Schema(description = "추천 기준(이른 시간 순 / 길게 볼 수 있는 순 / 최소 시간 보장 순)", example = "earliest")


최소 시간 보장은 추천 기준을 선택한 후 추가로 결정할 수 있는 하나의 옵션이라고 보는게 맞는 것 같아요.
함께 논의해봅시다.

저도 동의합니다. 정렬과 옵션을 구분하는 것이 좋을 것 같습니다~

함께 결정하고 싶었던 사안이라 우선 구현이 잘 보이는 분리된 추천으로 시작하였는데요.
스크럼에서 얘기 나눈대로 이 부분은 최소 시간 보장의 옵션으로 수정하도록 할게요!

hw0603

고생하셨습니다 다온!
정책 관련된 부분은 다른 팀원들이랑 전체 논의가 한 번 필요해 보이네요🤔

hw0603 · 2024-10-17T05:53:49Z

backend/src/main/java/kr/momo/domain/schedule/recommend/RecommendedScheduleSortStandard.java

@@ -9,7 +9,9 @@
 public enum RecommendedScheduleSortStandard {

    EARLIEST_ORDER("earliest", new EarliestFirstSorter()),
-    LONG_TERM_ORDER("longTerm", new LongTermFirstSorter());
+    LONG_TERM_ORDER("longTerm", new LongTermFirstSorter()),
+    MIN_TIME_EARLIER_ORDER("minTime", new EarliestFirstSorter())


최소 시간 보장 기능이 정렬의 기준이 된다기보다는 별도의 옵션으로 존재해야 하지 않을까요?

스크럼에서 얘기 나눴듯이 옵션으로 통일하겠습니다!

hw0603 · 2024-10-17T05:55:33Z

backend/src/main/java/kr/momo/controller/schedule/ScheduleController.java

+            @PathVariable String uuid,
+            @RequestParam String recommendType,
+            @RequestParam List<String> attendeeNames,
+            @RequestParam(value = "minTime", defaultValue = "0") int minTime
    ) {


기본값 설정👍

hw0603 · 2024-10-17T05:57:54Z

backend/src/main/java/kr/momo/controller/schedule/ScheduleControllerDocs.java

-            List<String> attendeeNames
+            List<String> attendeeNames,
+            @RequestParam @Schema(description = "최소 만남 시간(30분 단위의 분)", example = "0, 30, 60, 90")
+            int minTime


최소 만남 시간이 30분단위여야 할까요? 뭔가 사용자 입장에서는 한시간 단위로 두고 정수 1, 2 3 이렇게 전달하는게 편할 것 같아서요!

이 부분도 정책적인 사인이라 스크럼으로 얘기 나눴었죠.
시간 단위로 나누는 것으로 결론내었으니 반영해보도록 할게요!

hw0603 · 2024-10-17T06:00:29Z

backend/src/main/java/kr/momo/domain/schedule/recommend/CandidateSchedule.java

            idx += subList.size();
        }
        return mergedSchedules;
    }

+    private static void addWhenLongerOrEqualThanMinTime(
+            List<CandidateSchedule> subList, List<CandidateSchedule> mergedSchedules, int minSize


Suggested change

List<CandidateSchedule> subList, List<CandidateSchedule> mergedSchedules, int minSize

private static void addIfLongerThanOrEqualToMinTime(

일반적으로 사용되는 메서드 명명법을 따라 볼까요? 🙂

오호 이 부분은 흥미롭네요.
일반적으로 사용되는 메서드 명명법을 참고할 만한 곳이 있으면 공유 요청 드려도 될까요?
추후 메서드 명명에 도움이 될 것 같습니다!

음.. 별도로 레퍼런스가 있다기보다는 보통 영문에서 lt, lte, gt, gte 와 같은 표기를 많이 사용해서 그게 자연스럽다고 느꼈던 것 같아요.

자바 계열에서는 JUnit의 검증 메서드 명명법을 참고하기는 합니다. 저 메서드 보자마자 JUnit의 greaterThanOrEqualTo() 가 떠올랐어요ㅋㅋㅋ

hw0603 · 2024-10-17T06:04:50Z

backend/src/main/java/kr/momo/domain/schedule/recommend/CandidateSchedule.java

+        if (minSize < MINIMUM_MIN_SIZE) {
+            throw new MomoException(ScheduleErrorCode.INVALID_MIN_TIME);
+        }
+        if (minSize > subList.size()) {


minSize 검증의 책임은 SeheduleService에게 있지 않을까요?

동의합니다!
재즈 리뷰에도 질문 남겼지만 그러면 controller의 parameter 단에서 validation 라이브러리를 통해 검증하는건 어떨까요?

ehBeak

다온 구현하시느라 고생 많으셨습니다~

간단한 코멘트 남겨요 :)
추가로, minSize에 대한 네이밍을 조금 더 구체적으로 바꾼다면, 가독성이 더 좋아질 것 같아요~~!~!

ehBeak · 2024-10-17T12:26:29Z

backend/src/main/java/kr/momo/controller/schedule/ScheduleControllerDocs.java

@@ -98,9 +98,11 @@ MomoApiResponse<AttendeeScheduleResponse> findMySchedule(
    @ApiSuccessResponse.Ok("추천 일정 조회 성공")
    MomoApiResponse<RecommendedSchedulesResponse> recommendSchedules(
            @PathVariable @Schema(description = "약속 UUID") String uuid,
-            @RequestParam @Schema(description = "추천 기준(이른 시간 순 / 길게 볼 수 있는 순)", example = "earliest")
+            @RequestParam @Schema(description = "추천 기준(이른 시간 순 / 길게 볼 수 있는 순 / 최소 시간 보장 순)", example = "earliest")


저도 동의합니다. 정렬과 옵션을 구분하는 것이 좋을 것 같습니다~

ehBeak · 2024-10-17T12:41:17Z

backend/src/main/java/kr/momo/domain/schedule/recommend/RecommendedScheduleSortStandard.java

@@ -9,7 +9,9 @@
 public enum RecommendedScheduleSortStandard {

    EARLIEST_ORDER("earliest", new EarliestFirstSorter()),
-    LONG_TERM_ORDER("longTerm", new LongTermFirstSorter());
+    LONG_TERM_ORDER("longTerm", new LongTermFirstSorter()),
+    MIN_TIME_EARLIER_ORDER("minTime", new EarliestFirstSorter())


해당 클래스는 빠른 순, 오래 만나는 순처럼 정렬하는 로직에 관련된 것 같아요.
그러나 MIN_TIME_EARLIER_ORDER은 정렬보다는 필터에 가까운 느낌입니다. 해당 상수를 따로 관리하면 어떨까요?

최소 시간 보장이 '빠르게 만나요'의 옵션으로 정책이 확정되면서 MIN_TIME_EARLIER_ORDER이 필요하지 않아 삭제하였습니다!

ehBeak · 2024-10-17T12:55:47Z

backend/src/test/java/kr/momo/controller/schedule/ScheduleControllerTest.java

@@ -154,7 +154,7 @@ void findMySchedule() {
    void recommendSchedules() {
        RestAssured.given().log().all()
                .pathParam("uuid", meeting.getUuid())
-                .queryParams("recommendType", EARLIEST_ORDER.getType(), "attendeeNames", attendee.name())
+                .queryParams("recommendType", EARLIEST_ORDER.getType(), "attendeeNames", attendee.name(), "minimumTime", 0)


Suggested change

.queryParams("recommendType", EARLIEST_ORDER.getType(), "attendeeNames", attendee.name(), "minimumTime", 0)

.queryParams("recommendType", EARLIEST_ORDER.getType(), "attendeeNames", attendee.name())

이미 minimumTime의 기본값을 0으로 설정해 두었으니, 값을 설정하지 않아도 되겠네요 :)
만약 다온이 minimumTime 값을 의도하고 넣었다면, 개행만 부탁드릴게요~~

반영하였습니다!

hw0603

🚀

seunghye218

👍👍

ehBeak

고생 많으셨어요~!!!~~!~~!!!~~!!!!!!!!!🚀🚀

- 접근 제어자 수정 - 메서드 순서 컨벤션 적용 - 메서드 최대 길이 컨벤션 적용 - 불필요한 공백 제거

ikjo39 added 🐈‍⬛ 백엔드 백엔드 관련 이슈에요 :) 🚀 기능 기능을 개발해요 :) labels Oct 13, 2024

ikjo39 added this to the 6차(최종) 데모데이 milestone Oct 13, 2024

ikjo39 self-assigned this Oct 13, 2024

ikjo39 requested review from ehBeak, hw0603, seokmyungham and seunghye218 as code owners October 13, 2024 13:53

seokmyungham requested changes Oct 17, 2024

View reviewed changes

hw0603 requested changes Oct 17, 2024

View reviewed changes

ehBeak requested changes Oct 17, 2024

View reviewed changes

ikjo39 requested review from hw0603, ehBeak and seokmyungham October 19, 2024 11:41

seokmyungham approved these changes Oct 27, 2024

View reviewed changes

hw0603 changed the title ~~[BE] 최소 시간을 보장 추천 로직 추가~~ [BE] 최소 시간을 보장하는 추천 로직 추가 Oct 28, 2024

hw0603 approved these changes Oct 28, 2024

View reviewed changes

seunghye218 approved these changes Nov 1, 2024

View reviewed changes

ehBeak approved these changes Nov 5, 2024

View reviewed changes

ikjo39 added 9 commits November 6, 2024 15:40

refactor: 코드 스타일 및 컨벤션 적용

e600aea

- 접근 제어자 수정 - 메서드 순서 컨벤션 적용 - 메서드 최대 길이 컨벤션 적용 - 불필요한 공백 제거

feat: 최소 시간 보장 정렬 기준 추가

621ed1a

feat: 잘못된 최소 시간 입력시 예외 발생 로직 추가

42065aa

feat: 최소 시간 보장 추천 로직 추가

46e4631

refactor: 추천 요청 파라미터 값 객체화 및 최소 시간 검증 로직 추가

d7b0449

fix(ScheduleRecommendRequest): minTime 을 입력 받지 않은 경우 0으로 초기화

f3c7c29

refactor: 최소 시간 기준 수정 및 불필요한 정렬조건 삭제

ab70b8d

refactor: 가독성 있는 분기문 수정

f5955fd

refactor: 일반적으로 사용되는 메서드 명명법 수정

c848d79

ikjo39 force-pushed the feat/398-recommend-min-duration branch from 3769f63 to c848d79 Compare November 6, 2024 06:40

ikjo39 merged commit f41b097 into develop Nov 6, 2024
6 checks passed

ikjo39 deleted the feat/398-recommend-min-duration branch November 6, 2024 06:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BE] 최소 시간을 보장하는 추천 로직 추가 #404

[BE] 최소 시간을 보장하는 추천 로직 추가 #404

ikjo39 commented Oct 13, 2024 •

edited

Loading

github-actions bot commented Oct 13, 2024 •

edited

Loading

seokmyungham left a comment

seokmyungham Oct 17, 2024

ikjo39 Oct 19, 2024

seokmyungham Oct 20, 2024

seokmyungham Oct 17, 2024

ikjo39 Oct 19, 2024

seokmyungham Oct 17, 2024

ikjo39 Oct 19, 2024

seokmyungham Oct 17, 2024 •

edited

Loading

ehBeak Oct 17, 2024

ikjo39 Oct 19, 2024

hw0603 left a comment

hw0603 Oct 17, 2024

ikjo39 Oct 19, 2024

hw0603 Oct 17, 2024

hw0603 Oct 17, 2024

ikjo39 Oct 19, 2024

hw0603 Oct 17, 2024

ikjo39 Oct 19, 2024

hw0603 Oct 20, 2024

hw0603 Oct 17, 2024

ikjo39 Oct 19, 2024

ehBeak left a comment

ehBeak Oct 17, 2024

ehBeak Oct 17, 2024

ikjo39 Oct 19, 2024 •

edited

Loading

ehBeak Oct 17, 2024

ikjo39 Oct 19, 2024

hw0603 left a comment

seunghye218 left a comment

ehBeak left a comment

	List<CandidateSchedule> subList, List<CandidateSchedule> mergedSchedules, int minSize
	private static void addIfLongerThanOrEqualToMinTime(

	.queryParams("recommendType", EARLIEST_ORDER.getType(), "attendeeNames", attendee.name(), "minimumTime", 0)
	.queryParams("recommendType", EARLIEST_ORDER.getType(), "attendeeNames", attendee.name())

[BE] 최소 시간을 보장하는 추천 로직 추가 #404

[BE] 최소 시간을 보장하는 추천 로직 추가 #404

Conversation

ikjo39 commented Oct 13, 2024 • edited Loading

관련 이슈

작업 내용

최소 시간 기준

최소 시간 보장 로직

특이 사항

리뷰 요구사항

github-actions bot commented Oct 13, 2024 • edited Loading

Test Results

seokmyungham 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

seokmyungham Oct 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hw0603 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

Choose a reason for hiding this comment

ehBeak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ikjo39 Oct 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hw0603 left a comment

Choose a reason for hiding this comment

seunghye218 left a comment

Choose a reason for hiding this comment

ehBeak left a comment

Choose a reason for hiding this comment

ikjo39 commented Oct 13, 2024 •

edited

Loading

github-actions bot commented Oct 13, 2024 •

edited

Loading

seokmyungham Oct 17, 2024 •

edited

Loading

ikjo39 Oct 19, 2024 •

edited

Loading