⚙️ Refactoring 순원님이 만들어 놓으신 CusorPageResponse에 대한 의견 요청

[sikdong]

Issue: ⚙️ Refactoring

순원님이 만들어 놓으신 CusorPageResponse에 대한 의견 요청

Description

CursorPageResponse 로 변경 중 다른 도메인은 모르겠지만 제가 만든 Review 쪽에는 적용이 어려워 도움 요청 차 이슈를 남깁니다

적용이 어렵다고 생각하는 이유

1. Early Return의 불가

이번에 만들어주신 클래스는 첫번째 매개변수에 최종 dto(ReviewInfoResponse)를 담아야 하는데 ReviewInfoResponse는 Early Return이 되지 않는 경우(dto가 비어있지 않은 경우)에만 만들어지기 때문에 if -else 구문을 써보려고도 했지만 비어있는 경우 이후 로직을 안타는 것이 더 성능 상 좋겠다는 생각이 들었습니다

2. 해당 로직을 사용할 수 없음 보시는 것처럼 저는 ReviewCursor에서 범위를 정해 이미지와 도움되요를 가져오기 때문에 CursorPageResponse 를 적용하는 경우 이 로직을 사용 못하게 되더라구요.

이 이슈를 올린 이유는 다른 분들이 실제로 이 클래스를 사용했을 때 어떠셨는지 여쭤보고 싶었고 이 클래스는 사용하기 어렵다가 아닌 좀 더 같의 의논하면서 발전시켜보고 싶어서 올렸습니다. 의견 부탁 드립니다

To do

• [ ] todo

ETC

[yunyechan9893]

그러네요. 현재 로직은 11개를 가져온 후 다음 전처리 과정을 이행하기 위해 불필요한 아이템을 제거 (11개에서 10개로 만듦) 후 진행하는데, 순원님 클래스는 바로 Response로 만드니 문제가 될 수 있겠네요!

저도 내일 진행해보고 다시 댓글달겠습니다

[yunyechan9893]

동석님 이번 이슈 해결했습니다! 아래 PR 확인 부탁드릴게요

https://github.com/eco-dessert-platform/backend/pull/297

[sunwon12]

    @Transactional(readOnly = true)
    public CursorPageResponse<ReviewInfoResponse> getMyReviews(Long memberId, Long cursorId) {
        List<ReviewSingleDto> myReviewList = reviewRepository.getMyReviews(memberId, cursorId);
        if (ObjectUtils.isEmpty(myReviewList)) {
            return CursorPageResponse.of(Collections.emptyList(), 10, ReviewInfoResponse::getId);
        }

        List<Long> reviewIds = 리뷰아이디들가져오기(myReviewList);
        Map<Long, String> imagesMap = imageService.리뷰아이디들의_해당하는_이미지 가져오기(reviewIds);
        Map<Long, List<Long>> likeMap = makeLikeMap(reviewIds);

        List<ReviewInfoResponse> reviewInfoResponses = MappingReiew.reviewWithImageAndLike(myReviewList, imagesMap, likeMap);
        ReviewInfoResponse reviewInfoResponses2 = 태그액션으로_바꾸기(reviewInfoResponses); 

        return CursorPageResponse.of(reviewInfoResponses2, 10, ReviewInfoResponse::getId);
    }

이런 식으로 바꾸는 게 어떨까요? 그리고

1. CursorPageReponse가 생겼으므로 ReviewCursor 객체는 필요없을 것 같아요
2. 아이디 범위를 두어서 가져오기보단 아이디들에 해당하는 이미지나, 좋아요를 가져오는 게 어떨까요

+) 참고로 CursorPageResponse에 빈 DTO리스트 파라미터로 넘겨줘도 됩니다

[yunyechan9893]

Page 독립적으로 관리

• Cursor 기반의 페이지네이션을 DTO와 분리된 상태로 관리할 수 있도록 설계했기에, 사용자가 원하는 변환 로직을 Function으로 처리해 원하는 DTO나 인스턴스를 가공해 반환할 수 있도록 했습니다. (U를 도입한 이유)
• 이 방식으로 다양한 형태의 데이터 가공 및 커서 기반 페이지네이션 처리가 가능하며, 재사용성과 유지 보수성을 높히고자 했습니다

  
  package com.bbangle.bbangle.page;

import java.util.List; import java.util.function.Function; import java.util.function.ToLongFunction; import lombok.Getter; import lombok.RequiredArgsConstructor;

/**

• 커서 기반 페이지네이션을 사용하여 처리된 데이터를 담는 응답 클래스입니다.
• @param 페이지네이션 되는 개별 항목의 타입

• @param 변환 함수 적용 후 반환되는 처리된 데이터의 타입 */ @Getter @RequiredArgsConstructor public class ProcessedDataCursorResponse<T, U> {

  private static final Long NO_NEXT_CURSOR = -1L; private static final boolean NO_HAS_NEXT = false;

  private final U data; private final Long nextCursor; private final Boolean hasNext;

  /**

  • 주어진 데이터 리스트에 페이지네이션을 적용하여 새로운 {@link ProcessedDataCursorResponse}를 생성합니다. 변환 함수를 적용하여 처리된
  • 데이터를 반환합니다.
  • @param data 변환될 데이터 리스트
  • @param pageSize 페이지 크기 (한 페이지에 들어갈 항목의 개수)
  • @param idExtractor 항목에서 ID를 추출하는 함수 (커서로 사용)
  • @param func 데이터 리스트를 변환하여 처리된 데이터를 반환하는 함수

  • @return 처리된 데이터를 포함하는 {@link ProcessedDataCursorResponse} 객체 */ public static <T, U> ProcessedDataCursorResponse<T, U> of( List data, int pageSize, ToLongFunction idExtractor, Function<List, U> func ) { boolean hasNext = data.size() > pageSize; Long nextCursor = -1L;

    if (hasNext) { T lastReponse = data.get(pageSize - 1); nextCursor = idExtractor.applyAsLong(lastReponse); data = data.subList(0, pageSize); }

    U processedData = func.apply(data);

    return new ProcessedDataCursorResponse<>(processedData, nextCursor, hasNext); }

  public static <T, U> ProcessedDataCursorResponse<T, U> empty( U processedData) { return new ProcessedDataCursorResponse(processedData, NO_NEXT_CURSOR, NO_HAS_NEXT); } }

조합 사용

• 기존에는 페이지 상속의 방법으로 필요한 내용을 추가해서 구현해서 그 클래스 내부에서 구현이 가능했다면 두 개의 제네릭을 쓰는 과정에서 모든 로직이 그 안에서 해결되는 게 아니라면 저는 이전의 방식이 객체지향적인 방식이나 유지보수 측면에서 더 좋다는 생각이 들긴 합니다

@Getter
@RequiredArgsConstructor
@AllArgsConstructor
public class CustomPage<T> {

    private T content;
    private Long nextCursor;
    private Boolean hasNext;

}

@Getter
@NoArgsConstructor(access = AccessLevel.PRIVATE)
public class BoardCustomPage<T> extends CustomPage<T> {

    @JsonInclude(JsonInclude.Include.NON_NULL)
    private Long boardCount;
    @JsonInclude(JsonInclude.Include.NON_NULL)
    private Long storeCount;

    public BoardCustomPage(
        T content,
        Long requestCursor,
        Boolean hasNext,
        Long boardCount,
        Long storeCount
    ) {
        super(content, requestCursor, hasNext);
        this.boardCount = boardCount;
        this.storeCount = storeCount;
    }
}

결론

• 동석님 의견

  
  제가 정확한게 이해한 건지 모르겠지만
  중원님 입장은 기존 방식에서 변수 추가해서 각각 처리하는 조합 형식의 클래스로 사용하자
  예찬님은 아예 모든 경우를 대비해서 사용할 수 있는 U 변수를 가진 Resolver 클래스를 따로 만들어서 여기서 다 처리하자 
  인 거 같은데
  두 분 다 일리 있는 말씀이고 저도 취향차이라고 생각합니다
  다만 중원님 말씀대로 비슷한 역할 하는데 굳이 U를 추가해서 제한?을 둬야하냐는 데에 조금 더 동의하는 바입니다. 

처음 고민했던 부분이 추가적인 변수가 생겼을 때 순원님이 만들어주신 커스텀 클래스에 어떻게 적용할 것인가였기 때문에 이에 대해 생각해봤을때 크게 차이가 없다면 간단하게 가는게 좋지 않을 까라는 생각도 듭니다.

조금 정정 => 조금 정정을 하자면 U는 클래스이고, 확장도 둘 다 할 수 있습니다 (멤버 변수 둘 다 추가 가능)

다만, 해당 로직을 dto로 관리하느냐, 독립적인 객체로 dto를 래핑하여 관리하느냐의 차이입니다

현재는 둘 중 어떤걸 사용해도 상관없으므로 다수의 의견을 따르도록 하겠습니다~

- 순원님 의견

(동석님 표현에 의한)Resolver 클래스를 사용하더라도 중간에 DTO 클래스를 하나 만들어서 Resolver에게 넘겨줘야합니다.

commonCursorResponse를 만들려는 의도는

1. 클래스 수 줄이기
2. nextcursor, hasNext 관리 책임 CursorResponse에게 넘겨주기입니다.

예찬님 의견이랑 중원님의견이랑 2번은 해결합니다 그러나 둘 다 클래스 수를 줄이지는 못합니다.

그럼 다음 판단 기준으로는 더 직관적인 게 어떤 것이냐인데 중원님 의견의 코드가 더 직관인 것 같습니다

### 따라서 중원님 의견인 조합을 사용하기로 했습니다