Optimize KnockoutJs Pagination Computed Function

Question

I have written a function that builds an array of paging options for rendering on the view.

I have modelled it after DataTables for jQuery, a live example with lots of data is available here: http://www.companieshousedata.co.uk/a/490

Is there a better way to write this? The function also returns ellipsis (as strings) where necessary.

Model/object:

function PageDirection(data) {
    var self = this;
    self.display = data.display;
    self.value = data.value;        
    self.disabled = function (currentPage, lastPage) {
        return self.value() > lastPage || self.value() < 1 || self.value() == null;
    };
}

Computed observable:

self.pageDirections = ko.computed(function () {     
    var pageIndex = self.pageIndex();
    var lastPage = self.lastPage();

    //Previous
    var directions = [
        new PageDirection({
            display: "Previous",
            value: ko.computed(function() {
                return parseInt(pageIndex, 10) - 1;
            })
        })
    ];

    //6 or less pages
    if (self.lastPage() <= 6) {
        for (var i = 0; i < lastPage; i++) {
            directions.push(new PageDirection({
                display: i + 1,
                value: ko.observable(i + 1)
            }));
        }
    } else {
        if (pageIndex > 4) {
            //1
            directions.push(new PageDirection({
                display: 1,
                value: ko.observable(1)
            }));
            //...
            directions.push(new PageDirection({
                display: "...",
                value: ko.observable(null)
            }));

            if (lastPage - pageIndex <= 3) {
                //last 5
                for (var k = lastPage - 5; k < lastPage; k++) {
                    directions.push(new PageDirection({
                        display: k + 1,
                        value: ko.observable(k + 1)
                    }));
                }                   
            } else {
                //-1
                directions.push(new PageDirection({
                    display: pageIndex - 1,
                    value: ko.observable(pageIndex - 1)
                }));
                //current
                directions.push(new PageDirection({
                    display: pageIndex,
                    value: ko.observable(pageIndex)
                }));
            }

            if (lastPage - pageIndex > 3) {
                //+1
                directions.push(new PageDirection({
                    display: pageIndex + 1,
                    value: ko.observable(pageIndex + 1)
                }));
                //...
                directions.push(new PageDirection({
                    display: "...",
                    value: ko.observable(null)
                }));
            }
        } else {
            //first 5
            for (var l = 0; l < 5 ; l++) {
                directions.push(new PageDirection({
                    display: l + 1,
                    value: ko.observable(l + 1)
                }));
            }
            //...
            directions.push(new PageDirection({
                display: "...",
                value: ko.observable(null)
            }));

        }
        if (lastPage - pageIndex > 3) {
            //last
            directions.push(new PageDirection({
                display: lastPage,
                value: ko.observable(lastPage)
            }));
        }       
    }

    //Next
    directions.push(new PageDirection({
        display: "Next",
        value: ko.computed(function () {
            return parseInt(pageIndex, 10) + 1;
        })
    }));

    return directions;
});

View (for reference):

<span data-bind="foreach: pageDirections">
    <span data-bind="text: display, click: $parent.navigate.bind($parent), css: { 'inactive': disabled($parent.pageIndex(), $parent.lastPage()), 'active': $parent.pageIndex() == value() }"></span>
</span>

Sᴀᴍ Onᴇᴌᴀ · Accepted Answer · 2021-03-29 22:06:13Z

Let's take a look at that first line of the PageDirection function:

var self = this;

This is a common practice to keep a reference to the this object within the closure. A different way to do this would be to bind the context of the closure using Function.bind()

this.disabled = function (currentPage, lastPage) {
    return this.value() > lastPage || this.value() < 1 || this.value() == null;
}.bind(this);

However, there appears to be no need to bind the context

function PageDirection(data) {
    this.display = data.display;
    this.value = data.value;        
    this.disabled = function (currentPage, lastPage) {
        console.log('this.value: ', this.value(), ' > lastPage: ', this.value() > lastPage);
        return this.value() > lastPage || this.value() < 1 || this.value() == null;
    };
}

const pd1 = new PageDirection({value: _ => 16, display: 'BMW'});
console.log('pd1 disabled: ', pd1.disabled(1, 10));

Though maybe the KO code invokes that method differently, in which case the function may need to be bound.

Another option would be to use an ES6 arrow function expression, which doesn't have its own this binding:

this.disabled = (currentPage, lastPage) => 
                this.value() > lastPage || this.value() < 1 || this.value() == null;

To be honest, the logic in the observable looks a bit overly-complex. I didn't fully study it and grok it but I would look to see if some of the logic can be simplified - for example:

} else {
    if (pageIndex > 4) {

Unless it is the case that the subsequent logic wouldn't work you might consider if that can be converted to:

} else if (pageIndex > 4) {

Which could decrease the sub-sequent indentation levels.

Stack Exchange Network

Optimize KnockoutJs Pagination Computed Function

1 Answer 1

You must log in to answer this question.

Hot Network Questions

Optimize KnockoutJs Pagination Computed Function

1 Answer 1

You must log in to answer this question.

Related

Hot Network Questions