Github pagination

Question

I wanted to copy the behavior of github's pagination. Here is what it turned out: http://jsbin.com/dihohiseca/5/edit?js,output

It works fine, but I have a feeling that there is a lot of room for improvements. Please help me make my code more elegant / shorter.

var page = 12;
var numPages = 40;
var res = [];
var from = 1;
var to = numPages;
if (numPages > 10) {
 from = Math.max(page - 2, 1);
 to = Math.max(Math.min(page + 2, numPages), 5);
 if (to > 5) {
 res.push(1);
 if (from > 2) res.push(2);
 if (from > 3) {
 if (from === 4) {
 res.push(3);
 } else {
 res.push("...");
 }
 }
 }
}
for (var i = from; i <= to; i++) {
 res.push(i);
}
if (numPages > 10) {
 if (to < (numPages - 2)) {
 if (to === 8) {
 res.push(9);
 } else {
 res.push("...");
 }
 }
 if (to < numPages)
 res.push(numPages - 1);
 if (to !== numPages)
 res.push(numPages);
}
console.log(res);
$("body").html(res.toString());

<script src="https://ajax.googleapis.com/ajax/libs/jquery/1.11.0/jquery.min.js"></script>

The part with bigger page number was easy,

var page = 7;
var numPages = 155;
// [1, 2, "...", 5, 6, 7, 8, 9, "...", 154, 155]
//https://github.com/emberjs/ember.js/issues?page=7&q=is%3Aissue+is%3Aclosed

but with page number 11 the code got very messy.

var page = 5;
var numPages = 11;
// [1, 2, 3, 4, 5, 6, 7, "...", 10, 11]
// https://github.com/emberjs/ember.js/issues?page=7&q=is%3Aissue+is%3Aopen
var page = 6;
var numPages = 11;
// [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11]
// https://github.com/emberjs/ember.js/issues?page=6&q=is%3Aissue+is%3Aopen

Answer 1

I assume that page and numPages are the only variables you actually get, and the code is a snippet inside some function (if not, put it in one!). So first is... your code is pretty much imperative. Nothing wrong with it but the usual "issues" I see is some really nested if statements and a bunch of variables for tracking purposes. We'll take care of that in a bit.

Next would be variable naming. page doesn't tell me much. Is it the current page? I think so. So name it currentPage instead. Same with the other variables. Name them to describe what they're supposed to do. numPages? How about totalPages? res... what's that?

Anyways, here's my take on your code in a less if-else-ish manner. It isolates the logic down to the very last moment, in the ternary. If I placed a breakpoint just before that, I can easily see my variables and infer from them what happens next. Compare that to a deep if-else code where you have to step through the code in order to know how it ran, which branch it took this time etc.

function range(n){
 return Array.apply(null, Array(n)).map(function (_, i) {return i;});
}
function generatePagination(currentPage, totalPages){
 // First we move out the configurations. That way, they don't mingle with the logic.
 var initialChunkPadding = 1;
 var middleChunkPadding = 2;
 var endingChunkPadding = 1;
 var gapValue = '...';
 // Instead of a loop, we use range. It's much cleaner and we don't have tracking variables
 // at the cost of generating an array.
 return range(totalPages).reduce(function(pagination, index){
 // Then we determine what the current page is based on some comparisons
 var page = index + 1;
 var isInitialChunk = page <= 1 + initialChunkPadding;
 var isMiddleChunk = page >= currentPage - middleChunkPadding && page <= currentPage + middleChunkPadding;
 var isEndingChunk = page >= totalPages - endingChunkPadding;
 var hasNoGap = pagination[pagination.length - 1] !== gapValue;
 // Then based on the determinations, we determine what value gets pushed into the array.
 // It can either be the page, a '...', or a blank array (which doesn't change anything with concat)
 var valueToAdd = isInitialChunk || isMiddleChunk || isEndingChunk ? page : hasNoGap ? gapValue : [];
 
 return pagination.concat(valueToAdd);
 }, []);
}
// StackExchange should seriously add a console view >.<
document.write(JSON.stringify(generatePagination(16, 40)));
document.write('<br>');
document.write(JSON.stringify(generatePagination(7, 155)));
document.write('<br>');
document.write(JSON.stringify(generatePagination(5, 11)));
document.write('<br>');
document.write(JSON.stringify(generatePagination(6, 11)));

• user3568719: excellent solution and explanation, but I found a tiny mistake. generatePagination(1, 21) results: [1,2,3,"...",20,21], instead of [1,2,3,4,5,"...",20,21] .
• user3568719: And generatePagination(20, 21) results [1,2,"...",19,20,21] which should be [1,2,"...", 17, 18,19,20,21] see :github.com/strongloop/loopback/… (this is a bug in my code too)
• Joseph: @user3568719 Ahh, yep. That's a bug. The nice thing though is that the reduce is running through all page numbers. You can simply add a condition (I guess 2, one for the starting 5, and ending 5), and include them in the ternary that adds the page.