HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

What improvements can I make to this table filtering jQuery Plugin?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thiscanwhatmakepluginjqueryimprovementstablefiltering

Problem

This plugin is meant to filter through a table based on column header text that matches options given to it.
check it out here: Filter Table Plugin

Plugin code below for reference:

```
//framework from http://jqueryboilerplate.com/
; (function ($, window, document, undefined) {

var pluginName = "filterTable",
defaults = {
};
function Plugin(element, options) {
this.element = element;

this.options = $.extend({}, defaults, options);

this._defaults = defaults;
this._name = pluginName;

this.init();
}

Plugin.prototype = {

init: function () {

$el = $(this.element);
$table = $('#' + $el.data('filtertable'));
var filterColumnNames = ($el.attr('data-filtercolumns')).split(',');
var filterColumns = new Array();
var altRowCSS = $el.data('filteralternatingcss');

//get the column index of the column we'll be filtering on
$table.find('th').each(function (i) {
for (j in filterColumnNames) {
if ($(this).text().trim() == filterColumnNames[j]) {
filterColumns.push(i + 1);
}
}
});

$el.keyup(function () {
var query = $(this).val();
var numDisplayed = 0;
//remove any no result warnings
$table.find('#noResultsRow').remove();
//hide all rows
$table.find('tr').removeClass(altRowCSS).hide();
//show the header row
$table.find('tr:first').show();
for (i in filterColumns) {
$table.find('tr td:nth-child(' + filterColumns[i] + ')').each(function () {
if ($(this).text().toLowerCase().indexOf(query.toLowerCase()) >= 0) {
$(this).parent().show();
numDisplayed++;

Solution

-
You have:

this._defaults = defaults;


This is redundant as the declaration :

var pluginName = "filterTable",
    defaults = {
    };


Can be accessed anywhere in your plugin.

-
Declare all variables that you use. This is a really good practice to have change lines like:

$el = $(this.element);
 $table = $('#' + $el.data('filtertable'));


to:

var $el = $(this.element);
var $table = $('#' + $el.data('filtertable'));


JavaScript will let you use them without but you'll run into issues otherwise.

-
Variables need meaningful names so their lives don't seem meaningless and they despair.


Invective! Verb your expletive nouns!

$table.find('th').each(function (i)
{
    for (j in filterColumnNames)
    {
        if ($(this).text().trim() == filterColumnNames[j])
        {
             filterColumns.push(i + 1);
        }
    }
 });


What is i? index? columnIndex?

What is j? currentColumnName?

Code Snippets

this._defaults = defaults;
var pluginName = "filterTable",
    defaults = {
    };
$el = $(this.element);
 $table = $('#' + $el.data('filtertable'));
var $el = $(this.element);
var $table = $('#' + $el.data('filtertable'));
$table.find('th').each(function (i)
{
    for (j in filterColumnNames)
    {
        if ($(this).text().trim() == filterColumnNames[j])
        {
             filterColumns.push(i + 1);
        }
    }
 });

Context

StackExchange Code Review Q#24728, answer score: 2

Revisions (0)

No revisions yet.