Pagination algorithm in JS

Question

I needed a relatively simple pagination, as currently implemented on GoogleSearch (sans prev & next buttons). Most code I've seen was overly complicated so I came up with something that's simpler.

Any suggestions on how to further optimise this?

function doPaging(currentPageInput) {
 let currentPage = currentPageInput, // input
 range = 5, // amount of links displayed 
 totalPages = 20, // determined by amount of items, hardcoded for readability 
 start = 1; // default
 let paging = []; // output variable
 // Don't use negative values, force start at 1
 if (currentPage < (range / 2) + 1 ) {
 start = 1;
 // Don't go beyond the last page 
 } else if (currentPage >= (totalPages - (range / 2) )) {
 start = Math.floor(totalPages - range + 1);
 } else {
 start = (currentPage - Math.floor(range / 2));
 }
 for (let i = start; i <= ((start + range) - 1); i++) {
 if (i === currentPage) {
 paging.push(`[${i}]`); // add brackets to indicate current page 
 } else {
 paging.push(i.toString());
 }
 }
 return paging;
}

OUTPUT:

[ '[1]', '2', '3', '4', '5' ]
[ '1', '[2]', '3', '4', '5' ]
[ '1', '2', '[3]', '4', '5' ]
[ '2', '3', '[4]', '5', '6' ]
[ '3', '4', '[5]', '6', '7' ]
[ '4', '5', '[6]', '7', '8' ]
[ '5', '6', '[7]', '8', '9' ]
[ '6', '7', '[8]', '9', '10' ]
[ '7', '8', '[9]', '10', '11' ]
[ '8', '9', '[10]', '11', '12' ]
[ '9', '10', '[11]', '12', '13' ]
[ '10', '11', '[12]', '13', '14' ]
[ '11', '12', '[13]', '14', '15' ]
[ '12', '13', '[14]', '15', '16' ]
[ '13', '14', '[15]', '16', '17' ]
[ '14', '15', '[16]', '17', '18' ]
[ '15', '16', '[17]', '18', '19' ]
[ '16', '17', '[18]', '19', '20' ]
[ '16', '17', '18', '[19]', '20' ]
[ '16', '17', '18', '19', '[20]' ]

Answer 1

In addition to the remarks given by Blindman67, I'd like to suggest the following:

Passing options:

The first few lines of your function are some kind of configuration block:

let currentPage = currentPageInput, // input
 range = 5, // amount of links displayed 
 totalPages = 20, // determined by amount of items, hardcoded for readability 
 start = 1; // default

Now, when the number of pages changes, you need to touch your code and modify the totalPages initialization.

So you will probably want to introduce function arguments with default values:

function doPaging(currentPageInput, start = 1, totalPages = 20, range = 5) {
 ...
}
// Example:
doPaging(3, 1, 4); // ["0", "1", "2", "[3]", "4"]

Unfortunately, if you e.g. desire to change just the number of total pages, this argument order still requires you to specify all arguments preceding the totalPages argument.

Therefore, you might want to consider using default parameters in combination with destructuring assignment:

function doPaging(currentPageInput, {start = 1, totalPages = 20, range = 5} = {}) {
 ...
}
// Example:
doPaging(3, {totalPages: 4}); // ["0", "1", "2", "[3]", "4"]

Separating logic and view:

Your current implementation returns an array of strings destined for immediate output. This comes with a few drawbacks:

• If you want to change the markup of an active page index, you need to modify your code and replace the brackets [${i}] with something else.
• If you want to output a usable pagination, e.g. a list of anchor tags, you need additional code to map each page index string to its URL. This would probably require you to map each index string back to a number first.

So how about just returning the paging range as an array of numbers? Later on, within your HTML template, you can then print page == currentPage ? `[${page}]` : `${page}` or - in case you print HTML - simply select a different CSS class for the current page.

Handling corner cases:

In case you choose a range of 5, but the total number of pages is set to just 4, a call to doPaging(4) will return ["0", "1", "2", "3", "[4]"] including an undesired "0" at the beginning. A robust implementation should probably reduce the range according to the available total number of pages.

Fixing issues:

Since we now allow user defined start values, we need to fix issues arising from start values that are not 1 caused by the hard coded "force start at 1" within your code.

Putting it all together:

Choosing simpler and more consistent variable names as well as renaming the nondescript doPaging to getPagingRange, we get an easier to read and understand implementation.

Replacing the loop with Array.from yields more descriptive code.

Adding a single comment describing the function's behavior instead of many individual inline comments helps the reader to better understand the purpose and usage of the function.

/**
 * Return an integer range within [min, min + total) of given length centered
 * around the current page number.
 */
function getPagingRange(current, {min = 1, total = 20, length = 5} = {}) {
 if (length > total) length = total;
 let start = current - Math.floor(length / 2);
 start = Math.max(start, min);
 start = Math.min(start, min + total - length);
 
 return Array.from({length: length}, (el, i) => start + i);
}
// Examples:
console.log(getPagingRange(20)); // [16, 17, 18, 19, 20]
console.log(getPagingRange(3, {total: 4, length: 3})); // [2, 3, 4]
console.log(getPagingRange(3, {min: 0, total: 4, length: 3})); // [1, 2, 3]

• Robert: The declarations, inline comments and coupled logic&view were just for demo purposes and easy understanding of the code, but I appreciate you being thorough. Your solution is quite interesting. Although I find assigning different values to the same variable sequentially quite confusing to read, I do think that using Array.from() instead of a loop would make things more efficient. Also, thanks for finding the corner case!

Answer 2

Quick review

Some code style issues

• Use var, let, const appropriately.

• No need to hold a second reference to the variable currentPageInput

• When naming remember your context and stay consistent. You are in a function called doPaging and have variables start, range, then currentPage, totalPages post fixed. Either stick with the post fix startPage, displayPages, currentPage, totalPages or drop the postfix and have start, range, current, total with the preference for brevity IMHO.

• Too many comments. Comments are noise if you don`t need them to understand the code, and generally, if you need comments to understand the code, then the code needs refactoring, not comments.

• Don't use multiline declarations.

// bad
var a = 0,
 b = 1,
 c = 3;
// good
var a = 0;
var b = 1;
var c = 3;
// best
var a, b, c;
a = 0;
b = 1;
c = 3;

Code design.

You logic statements are a little inefficient. The majority of pages will use the final else block. That means most pages require you to divide range by 2 three times.

There is just too much repetition and noise. Consider using functions like Math.min and Math.max to clamp values rather than statements, For flooring positive numbers you can use | 0, and use ternary expressions for simple condition statements.

The function is impractical having the paging description hard coded. It would be much better to supply an object describing the page.

A rewrite

function doPaging(current, {range, pages, start = 1}) {
 const paging = []; 
 var i = Math.min(pages + start - range, Math.max(start, current - (range / 2 | 0)));
 const end = i + range;
 while (i < end) { paging.push(i === current ? `[${i++}]` : `${i++}`) }
 return paging;
}
// test
var i;
const paging = {range : 5, pages : 20};
for (i = 1; i <= 20; i ++) { console.log(doPaging(i, paging).join(",")) }

• JanErikGunnar: No doubt this version is computationally more efficient than the original and it certainly wins at code golf, however, concerning readability I find the original far better. For example, trying to figure out what "i" is supposed to be requires quite a bit of untangling. AT LEAST this version needs more line breaks, I would prefer it to have some more variables as well.
• le_m: Could you quickly comment on why you suggest separating variable declaration from initialization?
• Blindman67: @le_m Its the pattern of multi line declaration that is prone to error. Forgetting a , or mistakenly adding a ; can result in the creation of globals with the problem manifesting in totally unrelated code making the bug very hard to spot. Function scoped. var are hoisted automatically to the top of the function so it is good practice to declare them where they are declared, it reduces noise in the logic and also makes it easier to organise what variable names you have used if you have many.