1
\$\begingroup\$

I am writing a JavaScript feature where by I need to make a call to a third-party library (pre-1.10 DataTables) depending upon whether or not the user provides an integer as an input to the function during initialization. I am pasting here the snippet. As you can see it has duplicate code in the if/else clauses, the only difference is the method signature for *.fnFilter.

How can I optimize this code further? Also I dont want to test the condition during each key-up.

/*
 * By default this is a global search box events. Global being searchable across all columns.
 * But if an int is provided as an input then it will apply the search only on that column.
 * */
_bindSearchEvents: function(onColumnIndex) {

    var self = this;

    if (Math.ceil(onColumnIndex) === Math.floor(onColumnIndex)) {
        self.settings.search.input.live('keyup', function() {
            self.widget.fnFilter($(this).val(), onColumnIndex);
        });

        self.settings.search.reset.live('click', function() {
            self.settings.search.input.val("");
            self.widget.fnFilter("", onColumnIndex);
        });

    } else {

        self.settings.search.input.live('keyup', function() {
            self.widget.fnFilter($(this).val());
        });

        self.settings.search.reset.live('click', function() {
            self.settings.search.input.val("");
            self.widget.fnFilter("");
        });
    }
},
\$\endgroup\$
2
\$\begingroup\$

Based on http://legacy.datatables.net/api definition, the default integer value for .fnfilter(int) is null. It is recommended to set it to null rather than let it be undefined.

/*
 * By default this is a global search box events. Global being searchable across all columns.
 * But if an int is provided as an input then it will apply the search only on that column.
 * */
_bindSearchEvents: function (onColumnIndex) {

    var index = null;          // defaults to undefined
    if (Math.ceil(onColumnIndex) === Math.floor(onColumnIndex)) {
        index = onColumnIndex;
    }

    var searchInput = this.setting.search.input;
    var searchReset = this.setting.search.reset;
    var fnFilter = this.setting.widget.fnFilter;
    searchInput.live('keyup', function () {
        fnFilter($(this).val(), index);
    });

    searchReset.live('click', function () {
        searchInput.val("");
        fnFilter("", index);
    });
},

I use searchInput, searchReset, fnfilter to avoid repeat call to 'multi-layer functions' such as this.setting.search. This provide two benefits: i) More efficient in access input, reset and fnFilter. This could be a huge plus if the _bindSearchEvents is accessed multiple time. ii) Provide more readable code. This could be my own personal taste.

\$\endgroup\$
3
\$\begingroup\$

Since all that is different about the two if/else bodies is the arguments to the fnFilter method, you can construct the difference in one if statement and then use that in one copy of the code rather than two.

/*
 * By default this is a global search box events. Global being searchable across all columns.
 * But if an int is provided as an input then it will apply the search only on that column.
 * */
_bindSearchEvents: function (onColumnIndex) {

    var self = this;
    var args = [];
    if (Math.ceil(onColumnIndex) === Math.floor(onColumnIndex)) {
        args.push(onColumnIndex);
    }
    self.settings.search.input.live('keyup', function () {
        var argsCopy = [].concat(args);           // make copy of args
        argsCopy.unshift($(this).val());      // push item onto front of args
        self.widget.fnFilter.apply(self.widget, argsCopy);
    });

    self.settings.search.reset.live('click', function () {
        self.settings.search.input.val("");
        var argsCopy = [].concat(args);
        argsCopy.unshift("");                 // push item onto front of args
        self.widget.fnFilter.apply(self.widget, argsCopy);
    });
},

If the fnFilter function is written to check to see if the second parameter is null (rather than checking the number of arguments passed), then this shorter version could work also, but we'd have to see what the fnFilter documentation says or how the fnFilter source code actually works to know if this shorter method would work:

/*
 * By default this is a global search box events. Global being searchable across all columns.
 * But if an int is provided as an input then it will apply the search only on that column.
 * */
_bindSearchEvents: function (onColumnIndex) {

    var self = this;
    var index = null;
    if (Math.ceil(onColumnIndex) === Math.floor(onColumnIndex)) {
        index = onColumnIndex;
    }
    self.settings.search.input.live('keyup', function () {
        self.widget.fnFilter($(this).val(), index);
    });

    self.settings.search.reset.live('click', function () {
        self.settings.search.input.val("");
        self.widget.fnFilter("", index);
    });
},
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.