Pagination calculation

Question

I have written a function to get the actual page range to display that page range in pagination.

My function is working properly. But I think that I have written bad code. Can someone review my code and give me some feedback to improve my code?

Here is the code:

const pageRange = (() => {
 let startPage = activePage < pageNeighbours + 1 ? 1 : activePage - pageNeighbours;
 const endPage = totalPages < (pageNeighbours * 2) + startPage
 ? totalPages : (pageNeighbours * 2) + startPage;
 const diff = (startPage - endPage) + (pageNeighbours * 2);
 startPage -= (startPage - diff > 0 ? diff : 0);
 let actualPageRange = new Array((endPage - startPage) + 1)
 .fill().map((_, i) => i + startPage);
 if (actualPageRange[0] !== 1) actualPageRange = [1, ...actualPageRange];
 if (actualPageRange[1] !== 2) actualPageRange = [1, '...', ...actualPageRange.slice(1)];
 if (actualPageRange[actualPageRange.length - 1] !== totalPages) {
 actualPageRange = [...actualPageRange, totalPages];
 }
 if (actualPageRange[actualPageRange.length - 2] !== totalPages - 1) {
 actualPageRange = [...actualPageRange.slice(0, actualPageRange.length - 1), '...', totalPages];
 }
 return actualPageRange;
})();

In the above code:

activePage refers to the current page number.

pageNeighbours refers to the number of pages that should be shown besides active page

totalPages is self-explanatory

Update

After this function execution, I will use pageRange to create output like:

enter image description here

• konijn: Greetings, please roll back your changes (the updated code part). On this site, one should not update the question because of provided answers. Though the explanation of your variables helps and should be kept.
• Heslacher: Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers .
• Vishal: @konijn and Heslacher Sorry, I was not aware of that. Since Heslacher and konijn has already removed the update, I don't have to make any changes. Thank you. I will never do that again.

Answer 1

From a short review;

• You use globals instead of parameters for activePage and pageNeighbours
• You need at least one comment stating what pageNeighbours stands for, without knowing this the code looks meaningless to me
• You need at least one comment on the top explaining what the range is supposed to contain
• On the whole I think this could use more comments
• Why actualPageRange instead of simply pageRange, or even range since it would be clear from context what range we're reading about

Okay, so since the code does not work, expect this question to be closed. That doesn't mean that functionally this isn't a great question.

This is my approach to what I believe you are looking for:

const pageRange = ((activePage, pageNeighbours, totalPages) => {
 let range = [];
 //Add active page and neighbouring pages
 for(let page = activePage - pageNeighbours; page <= activePage + pageNeighbours; page++)
 range.push(page);
 //Make sure we dont show pages that dont exist
 range = range.filter(page=> page > 0 && page <= totalPages);
 //Allow user to go to the first page if need be
 //The second entry should be either 2 or ellipsis(...)
 if(range[0]!=1){
 if(range[1]!=2){
 range.unshift('...');
 }
 range.unshift(1);
 }
 //Allow user to go the last page, second last entry should be second last page or ellipsis
 if(range[range.length-1] != totalPages){
 if(range[range.length-2]!= totalPages-1){
 range.push('...');
 } 
 range.push(totalPages);
 }
 return range;
})(activePage, pageNeighbours, totalPages);

Note that this avoids accessing activePage etc. as a global.

• Vishal: Thank you konijn. I will try to improve my code as per your suggestions. Actually, I am using React.js, where the function is defined in local scope. So, activePage and pageNeighbours are the props of the component. Since, my function is defined inside the component, this function does not need both of them as parameters. pageNeighbours property is used to decide how many pages should be shown besides active page number.
• Vishal: I didn't notice an update in your question. I think that's a bug in my code. It should not print 4 in the first case. second case looks good though.
• Vishal: It's not arbitrary. It should return [ 1,...,5,6,7,8,9,10 ] in the first case. While it should return [ 1,...,6,7,8,9,10 ] in the second case. I will update the question with the image in a moment.
• Vishal: Please look at the update part of the question.
• Vishal: yes its true. There are some bugs. I never knew there were such bugs, that you found. I will try to fix them and then will give you the updated code. Most probably there is only 1 bug that I can see. If you have items less than pageNeighbours on the left of your current page, then it will add 1 more on the right side. Similar for vice-versa. I will fix that soon.