0
\$\begingroup\$

I implemented a function for pager link creation, it looks like this:

function createPagerObject(pageUri, page, totPages) {

    const MAX_PAGES_IN_PAGER = 6;
    // for the first three pages the starting would be 1
    let pageNumber = Math.sign(page - 3) > 0 ? page - 3 : 1;

    let pager = [];
    for(let i = 0; i < MAX_PAGES_IN_PAGER; i++){
        pager.push({
            "page": pageNumber,
            "href": pageUri.replace("list-" + page, "list-" + pageNumber),
            "type": pageNumber == page ? "current" : pageNumber > page ? "next" : "prev"
        });
        pageNumber++;
        if(pageNumber > totPages) {
            break;
        }
    }

    return pager;
}

the code is self explanatory, but the idea is to create an Array of objects with the structure:

page → the page number
href → the link to the destination page
type → "current", "next" or "prev" depending on the current page

having as input the current page URI, the current page number and the number of pages.

The question is "it could be improved", because it seems not so clean to me.

\$\endgroup\$
  • \$\begingroup\$ Where do you want to implement this paging? \$\endgroup\$ – INDIA IT TECH Mar 25 '16 at 10:36
  • \$\begingroup\$ Inside an Api in the frontend backend, it would be retourned in a complex object containing data for a React.js server rendered page \$\endgroup\$ – Daniele Mar 25 '16 at 10:44
2
\$\begingroup\$

Assuming ES6 is available (you tagged ES6, you have let and const, etc.), this can be simplified.

First, you can compute ahead the number of pages you want to include. You don't need to loop through to check. In this case, your loop depends on who is lesser between MAX_PAGES_IN_PAGER and totPages. You can use Math.min.

const limit = Math.min(totPages, MAX_PAGES_IN_PAGER);

Next, instead of loops and push, you can use array methods. Let's start off by taking out the loop. To loop n times with arrays, you can create a filled array with n dummy items. This is possible with Array(n).fill(0). Then, instead of push, we use map. We turn the n dummy values into values we need.

return Array(limit).fill(0).map(_ => ({
  page: ...,
  href: ...,
  type: ...
}));

In other news, use template strings to construct strings instead of concatenating. Note that template strings use backticks instead of quotes.

pageUri.replace(`list-${page}`, `list-${pageNumber}`)

Ternaries are hard to read. However, there's a way to keep them readable. What you are doing is right, putting further conditions at the false end of the ternary. They can be written in a cascading manner for readability.

"type": pageNumber == page ? "current"
       : pageNumber > page ? "next" 
                           : "prev"

Lastly, name your variables properly. totPages is confusing because "tot" doesn't tell me what it is. pager is confusing because it sounds like an object with methods, when it's just an array of objects representing pages.

Also, quotes on object keys are optional unless you are using key names with invalid characters.

In the end:

function createPageList(pageUri, page, totalPages) {

  const limit = Math.min(totalPages, 6);

  return Array(limit).fill(0).map((fill, index) => {

    const pageNumber = (page < 3 ? 1 : page - 3) + index ;

    return {
      page: pageNumber,
      href: pageUri.replace(`list-${page}`, `list-${pageNumber}`)
      type: pageNumber == page ? "current"
           : pageNumber > page ? "next" 
                               : "prev"
    };
  });
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.