Pagination logic created from scratch

Question

I created this JavaScript class using ES6-ES7 syntax to return an array of page numbers and ellipsis points (null) to be consumed by a function which will create a styled paginator.

What I am looking for in a code review is how the code could be improved for optimal readability, improved for speed, and simplified if possible (without loosing readability).

Example use:

(new Paginator({totalPages: 100, currentPage: 50})).execute()

This will return [1, null, 48, 49, 50, 51, 52, null, 100]

export class PaginationStrategy {
 constructor(params) {
 this.params = {
 totalPages: params.totalPages,
 currentPage: params.currentPage,
 };
 this.constants = {
 firstPage: 1,
 edgeSeqLen: 7,
 midSeqLen: 5, // must be odd
 breakPoint: 5,
 unbrokenPoint: 9,
 };
 // Label is the name of the action (Action Label)
 // Value is the name of the action handler (Action Function)
 this.actions = {
 noEllipsis: 'noEllipsis',
 focusBetweenEllipsis: 'focusBetweenEllipsis',
 focusBeforeOrAfterEllipsis: 'focusBeforeOrAfterEllipsis',
 };
 }
 /**
 * Execute
 *
 * Command to activate both the strategy and action
 * Note: Should be the only function one would need to call on this object
 */
 execute() {
 // Strategy
 const actionType = this.strategyInterface;
 // Action
 const list = this.actionInterface(actionType);
 // Result
 return list;
 }
 /**
 * Strategy Interface
 *
 * The logic to decide which action to take
 * - In other words which display logic to choose
 *
 * Notes: Break point refers to an ellipsis Ex: (1 2 3 ... 5).
 * Scope is the list of pages including ellipsis, as well
 * as the focus which is the page which the user is on
 */
 get strategyInterface() {
 if (this.hasBreakPoint) {
 if (this.isFocusInbetweenBreakPoints) {
 return this.actions.focusBetweenEllipsis;
 } else {
 return this.actions.focusBeforeOrAfterEllipsis;
 }
 } else {
 return this.actions.noEllipsis;
 }
 }
 // Does the paginator have a break point (ellipsis)?
 get hasBreakPoint() {
 const { totalPages } = this.params;
 const { unbrokenPoint } = this.constants;
 return (totalPages > unbrokenPoint)
 }
 // Is the focus between break points (ellipsis)?
 get isFocusInbetweenBreakPoints() {
 return this.isFocusAfterFirstBreakpoint && this.isFocusBeforeEndBreakpoint
 }
 get isFocusAfterFirstBreakpoint() {
 const { currentPage } = this.params;
 const { breakPoint } = this.constants;
 return (currentPage > breakPoint)
 }
 get isFocusBeforeEndBreakpoint() {
 const { currentPage, totalPages} = this.params;
 const { breakPoint } = this.constants;
 return currentPage <= (totalPages - breakPoint)
 }
 /**
 * Action Interface
 *
 * The display logic
 *
 * Note: The action to take should be generated by the strategy.
 * To add and remove actions create a function below, and add
 * the function name to the value of the actions object on this
 * objects prototype (The key should be descriptive as it is
 * used to get the actions function name in the strategy)
 */
 actionInterface(actionType) {
 if(!this[actionType]) return console.error(`Action: ${actionType} does not exist, maybe you should write some tests`);
 return this[actionType]();
 }
 // Focus is between ellipsis (break points)
 // this.actions['focusBetweenEllipsis']
 focusBetweenEllipsis() {
 const { totalPages, currentPage } = this.params;
 const { midSeqLen, firstPage } = this.constants;
 const amplitude = (midSeqLen - 1) / 2; // AKA max distance from center
 const middle = this.sequencer(midSeqLen, (idx) => (idx - amplitude) + currentPage);
 return this.flattener([firstPage, null, middle, null, totalPages]);
 }
 // Focus is before or after a single ellipsis (break point)
 // this.actions['focusBeforeOrAfterEllipsis']
 focusBeforeOrAfterEllipsis() {
 const { totalPages, currentPage } = this.params;
 const { breakPoint, edgeSeqLen, firstPage } = this.constants;
 const first = (currentPage <= breakPoint)
 ? this.sequencer(edgeSeqLen, (idx) => idx + 1)
 : [firstPage];
 const last = (currentPage >= (totalPages - breakPoint))
 ? this.sequencer(edgeSeqLen, (idx) => totalPages - ((edgeSeqLen - 1) - idx))
 : [totalPages];
 return this.flattener([first, null, last]);
 }
 // There is no ellipsis (break point)
 // this.actions['noEllipsis']
 noEllipsis() {
 const { totalPages } = this.params;
 return this.sequencer(totalPages, (idx) => idx + 1);
 }
 /**
 * Action Helpers
 *
 * Shared logic between the actions
 */
 sequencer(length, expression) {
 return Array.apply(0, Array(length)).map( (curr, idx) => expression(idx) )
 }
 flattener(arrays) {
 return [].concat.apply([], arrays)
 }
}

• Pavlo: You seem to have included Ramda (R), but have used it only once. Is this something you want to avoid? If not, you can make the code shorter by utilizing R methods.
• Mike: Yah, I could use an array flattening shim @Pavlo . Shouldn't be an issue let me look into that now.
• Mike: The code no longer has outside dependencies, if you wish to run the code I would run it through the babeljs.io/repl as it can handle ES6-ES7 syntax which is not available in the browser or in Node version 5

Answer 1

Disclaimer: I am not very familiar with the Ecmascript 6 class syntax.

Styling and readability

You are omitting the semicolon after most return-statements. I recommend putting a semicolon after every statement, even though a newline character will close the return statement.

Consider following these naming conventions.

"execute" is not a very descriptive name for a function. It outputs a formatting array, so a possible alternative could be to name it "format".

Semantics

You define this.constants as a public mutable property. You can consider at least using Object.freeze (mdn).

I do not like the following syntax. It is for the logic in the rest of the class irrelevant if, for example, totalPages was defined by the user or a pre-defined constant. If you decide to let the user define, for example, the starting page too in the future, you suddenly need to go through the entire code and change from where the variable is loaded. Instead, I would suggest just using properties such as this.firstPage which is either the user-defined value, or a default value.

constructor(params) {
 this.params = {
 totalPages: params.totalPages,
 currentPage: params.currentPage,
 };
 //...
}

Note: I am assuming you have renamed your class to Paginator since your usage example shows this name. Your class name PaginationStrategy does not really define what the class is supposed to do.

Over-engineering

For code that is supposed to output a simple list that could be generated with a range-generator (more or less your sequencer), 4 additions and 4 comparisons, or a little more if you want to keep the length constant, this class seems highly over-engineered. All these 'strategies' and 'actions' do not seem to add anything other than making the code execution jump all over the place, while most of it can just be generalized.

export class Pagination {
 constructor(params) {
 this.defaultValues = {
 FIRST_PAGE: 1,
 SEQUENCE_LENGTH: 9, //Must be at least 5
 STEP_SIZE: 1,
 };
 if(Object.freeze) {
 Object.freeze(this.defaultValues);
 }
 this.firstPage = this.defaultValues.FIRST_PAGE;
 this.lastPage = params.totalPages;
 this.currentPage = params.currentPage;
 this.sequenceLen = this.defaultValues.SEQUENCLE_LENGTH;
 this.stepSize = this.defaultValues.STEP_SIZE;
 }
 format() {
 //Generate these sequences:
 //1. [this.firstPage]
 //2. [null] or []
 //3. Sequence starting at
 // max( this.firstPage + this.stepSize, this.currentPage - offsetLeft )
 // where offsetLeft is min( based on sequenceLength, based on lastPage )
 // ending similarly
 //4. [null] or []
 //5. [this.lastPage]
 }
 range(start, end, stepSize) {
 const length = Math.floor( (end - start) / stepSize );
 return Array.apply(0, Array(length)).map( (curr, idx) => (start + (idx * stepSize)) );
 }
}

• Pavlo: A brilliant point on overengineering! To push it even further: this.defaultValues could be just a bunch of constants, i.e. const FIRST_PAGE = 1. Also, ES2015 goodies: constructor(params) could be constructor({ totalPages, currentPage }); Array.apply(0, Array(length)) could be Array(length).fill().