pagination 간단한 피드백 부탁드립니다~

JavaScript
1

안녕하세요 부쩍 클래스로 상태관리 하는것에 관심이 많아져 pagination을 JS class로 간단하게 만들어봤습니다.

몇 줄 안되는 코드지만 초보고 익숙하지 않으니 이마저도 오래걸린듯 합니다…:sob:

혹시 시간이 되신다면 간단하게 피드백 부탁드려도 될까요? 짧아도 괜찮습니다! (코드의 중복, 개선점 ,역할 분리 등등)

모바일로는 잘 표시되지 않는점 양해부탁드립니다

링크 : Pagination

코드 : pagination-oop/pagination.js at master · juo1221/pagination-oop · GitHub

혹은 일정한 비용을 내면 코드리뷰 받을 수 있는곳이 있을까요?? 감사합니다

2

코드 적으신 거 보고 재밌어 보여서 의견 좀 적어 봅니다.

Pagination 클래스를 보시면

#setParams({ currentPage, totalItemCount, pagePerItemCount } = {}, base) {
    const {
      currentPage: basePage,
      totalItemCount: baseItemCount,
      pagePerItemCount: basePageCount,
    } = base;
    this.currentPage = this.#chCurrPage(currentPage) || basePage;
    this.totalItemCount = this.#chTotalItemCnt(totalItemCount) ?? baseItemCount;
    this.pagePerItemCount = this.#chPageCtn(pagePerItemCount) || basePageCount;
  }
  #check(arg, bool) {
    if ((bool ? arg >= 0 : arg > 0) && typeof arg === 'number') return arg;
    else return null;
  }
  #chCurrPage(currentPage) {
    return this.#check(currentPage, 1);
  }
  #chTotalItemCnt(totalItemCount) {
    return this.#check(totalItemCount, true);
  }
  #chPageCtn(pagePerItemCount) {
    return this.#check(pagePerItemCount, 2);
  }

이렇게 코드가 있는데, 이 부분에서 #chCurrPage, #chTotalItemCnt, #chPageCtn은 굳이 필요한가 싶습니다. #check(arg, bool)의 bool 인수도 역할이 무엇인지 이해가 되지 않고요.

#setParams({ currentPage, totalItemCount, pagePerItemCount } = {}, base) {
    const {
      currentPage: basePage,
      totalItemCount: baseItemCount,
      pagePerItemCount: basePageCount,
    } = base;
    this.currentPage = this.#check(currentPage) || basePage;
    this.totalItemCount = this.#check(totalItemCount) ?? baseItemCount;
    this.pagePerItemCount = this.#check(pagePerItemCount) || basePageCount;
  }
  #check(arg) {
    if (typeof arg === 'number' && arg >= 0) return arg;
    else return null;
  }

이렇게만 적어도 기존 코드와 동작은 같으니 이렇게 하면 더 나으리라 생각합니다.

3

아 그러네요 굳이 메서드를 생성해서 나눌필요없이 check 하나로 하는게 더 좋아 보입니다.

bool 인수같은 경우엔 Pagination 제작 조건에서 totalItemCount은 0이 들어올 수 있기 때문에 arg>=0 이 적용돼야 하지만 나머지는 0이 들어올 수 없기 때문에 arg > 0으로 적용돼야 해서 생성했습니다.

제가 테스트하느라 chCurrPage와 chPageCtn의 2번째인자에 숫자를 넣었는데 그걸 삭제하지 않고 올려버려서 혼란을 야기한것 같습니다 ㅎㅎㅎ…

#setParams({ currentPage, totalItemCount, pagePerItemCount } = {}, base) {
    const {
      currentPage: basePage,
      totalItemCount: baseItemCount,
      pagePerItemCount: basePageCount,
    } = base;
    this.currentPage = this.#check(currentPage, null) || basePage;
    this.totalItemCount = this.#check(totalItemCount, true) ?? baseItemCount;
    this.pagePerItemCount = this.#check(pagePerItemCount, null) || basePageCount;
  }
  #check(arg ,bool) {
 if ((bool ? arg >= 0 : arg > 0) && typeof arg === 'number') return arg;
    else return null;
  }

이렇게 하면 될거 같네요…!

답변 감사드립니다!

4

사실 굳이 #check 메소드의 bool인자를 넣지 않아도 코드 흐름상 작동하는 데에는 문제 없습니다.

this.currentPage = this.#check(currentPage, null) || basePage;
this.totalItemCount = this.#check(totalItemCount, true) ?? baseItemCount;
this.pagePerItemCount = this.#check(pagePerItemCount, null) || basePageCount;

여기서 ||랑 ??는 비슷한 역할을 하지만 엄밀하게 작동하는 이유가 다릅니다.
??의 경우에는 일반적으로 생각되는 null이나 undefined일 때만 다음 피연산자를 반환하지만,
||의 경우에는 첫번째 피연산자가 false로 간주될 수 있는 값이면 다음 피연산자를 반환합니다.

0과 3을 각각의 연산자에 넣으면 0 ?? 3은 0이 반환되고, 0 || 3은 3이 반환되는 것을 알 수 있습니다.

그렇기 때문에

this.#check(currentPage, null) || basePage와 this.#check(pagePerItemCount, null) || basePageCount은 0일 경우 값이 대체되겠지만, this.#check(totalItemCount, true) ?? baseItemCount의 경우에는 this.#check(totalItemCount, true)이 0이어도 값의 교체가 일어나지 않을 겁니다.

전 그걸 의도하시고 코드 짜신 것으로 이해했지 말입니다. ㅎㅎ

5

아이고 맞네요 제가 그걸 의도해놓고 괜히 bool을 건드리고 있었네요 ㅋㅋㅋㅋ 좋은 지적 감사합니다!