Skip to content

INTERNAL: Change logic in getSubList method. #651

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

Merged
merged 1 commit into from
Aug 9, 2023

Conversation

brido4125
Copy link
Collaborator

Motivation

smget 연산의 최종결과를 계산할 때 사용되는 getSubList의 로직을 변경한다.
기존의 로직보다 간결한 로직으로 수정하여 가독성을 증가시킨다.

변경 사항

  • 삼항연산자 -> Math.min 사용
  • 조건문 Depth 감소

@brido4125 brido4125 self-assigned this Aug 8, 2023
@brido4125 brido4125 requested a review from uhm0311 August 8, 2023 03:49
@jhpark816 jhpark816 requested a review from oliviarla August 8, 2023 10:48
@jhpark816
Copy link
Collaborator

@uhm0311 @oliviarla 간단한 코드입니다. 리뷰 바랍니다.

Copy link
Collaborator

@jhpark816 jhpark816 left a comment

Choose a reason for hiding this comment

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

리뷰 완료

}
return mergedResult;
Copy link
Collaborator

Choose a reason for hiding this comment

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

아래와 같이 간단히 구현할 수 있습니다.

    if (offset > 0) {
      if ((offset+count) < mergedResult.size()) {
        return mergedResult.subList(offset, (offset+count));
      }
      if (offset < mergedResult.size()) {
        return mergedResult.subList(offset, mergedResult.size());
      }
      return Collections.emptyList();
    } else {
      if (count < mergedResult.size()) {
        return mergedResult.subList(0, count);
      }
      return mergedResult;
    }

@brido4125 brido4125 force-pushed the refactor/getSubList branch from 5791d69 to b2ed6b2 Compare August 9, 2023 01:29
@jhpark816 jhpark816 merged commit 7960754 into naver:develop Aug 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants