Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

I have made a simple pagination algorithm. It shows "next" and "previous" buttons when appropriate.

It also limits the amount of buttons if the page amount exceeds a certain amount, e.g. 1 2 ... 15 16 17 18 19 ... 99 100.

It works fine, but I feel it can be simplified.

function appendTruncateButtons(buttons, pageCount) {
  buttons.push("...");
  buttons.push(pageCount - 1);
  buttons.push(pageCount);
}

function prependTruncateButtons(buttons) {
  buttons.push(1);
  buttons.push(2);
  buttons.push("...");
}

function generateButtons(currentPage, pageCount, surroundingPageLinkSize) {
  var buttons = [];

  if (currentPage > 1) {
    buttons.push("<");
  }

  var truncate = pageCount > (7 + (surroundingPageLinkSize * 2));
  if (!truncate) {
    for (var i = 1; i <= pageCount; i++) {
      buttons.push(i);
    }
  }
  else {
    var truncateEnd = currentPage < (1 + (surroundingPageLinkSize * 2)),
        truncateBoth = (pageCount - (surroundingPageLinkSize * 2)) > currentPage && currentPage > (surroundingPageLinkSize * 2);

    if (truncateEnd) {
      for (var j = 1; j < (4 + (surroundingPageLinkSize * 2)); j++) {
        buttons.push(j);
      }
      appendTruncateButtons(buttons, pageCount);
    }
    else if (truncateBoth) {
      prependTruncateButtons(buttons);
      for (var k = (currentPage - surroundingPageLinkSize); k <= (currentPage + surroundingPageLinkSize); k++) {
        buttons.push(k);
      }
      appendTruncateButtons(buttons, pageCount);
    }
    else { // Truncate start
      prependTruncateButtons(buttons);
      for (var l = pageCount - (2 + (surroundingPageLinkSize * 2)); l <= pageCount; l++) {
        buttons.push(l);
      }
    }
  }

  if (currentPage < pageCount) {
    buttons.push(">");
  }

  return buttons;
}
// generateButtons(currentPage, pageCount, surroundingPageLinkSize)
console.log(generateButtons(1, 100, 3));
console.log(generateButtons(50, 100, 3));
console.log(generateButtons(99, 100, 3));

share|improve this question
up vote 2 down vote accepted
  var truncate = pageCount > (7 + (surroundingPageLinkSize * 2));
  if (!truncate) {
    for (var i = 1; i <= pageCount; i++) {
      buttons.push(i);
    }
  }
  else {
    var truncateEnd = currentPage < (1 + (surroundingPageLinkSize * 2)),
        truncateBoth = (pageCount - (surroundingPageLinkSize * 2)) > currentPage && currentPage > (surroundingPageLinkSize * 2);

    if (truncateEnd) {
      for (var j = 1; j < (4 + (surroundingPageLinkSize * 2)); j++) {
        buttons.push(j);
      }
      appendTruncateButtons(buttons, pageCount);
    }
    else if (truncateBoth) {
      prependTruncateButtons(buttons);
      for (var k = (currentPage - surroundingPageLinkSize); k <= (currentPage + surroundingPageLinkSize); k++) {
        buttons.push(k);
      }
      appendTruncateButtons(buttons, pageCount);
    }
    else { // Truncate start
      prependTruncateButtons(buttons);
      for (var l = pageCount - (2 + (surroundingPageLinkSize * 2)); l <= pageCount; l++) {
        buttons.push(l);
      }
    }
  }

Note that all of these cases have essentially the same for loop.

  var start = 1,
      end = pageCount;

  if (currentPage > surroundingPageLinkSize * 2) {
      start = currentPage - surroundingPageLinkSize;
      prependTruncateButtons(buttons);
  }

  if (currentPage < pageCount - surroundingPageLinkSize * 2 - 2) {
      end = currentPage + surroundingPageLinkSize;
  }

  for (var i = start; i <= end; i++) {
      buttons.push(i);
  }

  if (end < pageCount) {
      appendTruncateButtons(buttons);
  }

Now we call it just once, only changing the parameters.

We also only call the prepend and append methods once each. You could even pull those into this method if you wanted, as they are no longer repeated code.

I also like that this gets rid of the three single use Boolean variables. Since they weren't reused, they don't seem necessary.

I'm not sure if the output is an exact match for what you had originally, but it should give the same kind of display.

I didn't change it here, but consider changing buttons to pages or even pageNumbers. These are the pages that you want to make links or buttons that go there. But they aren't buttons yet.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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