CODEWITHSUNDEEP
javascriptjqueryhtml
Reily Bourne
Reily Bourne

Reputation: 5307

Javascript/Jquery filters on HTML table is highly inefficient

By inefficient I mean, activating the code makes the page hang for 20+ seconds.

To set the scene, I currently have an HTML table that looks like the following. It can be fairly big, easily 1,000-1,500 rows and 40 columns wide. It is generated from Python/Flask as a static HTML table and then javascript takes over to allow the users to filter out and sort rows. I do use the jquery tablesorter widget to allow users to sort rows by whatever column they wish.

The table itself has a structure like:

<table id="myTablePlayers" class="tablesorter table table-striped table-bordered table-hover" style="overflow: visible">
    <thead>
        <tr>
          <th>...</th>
          <th>...</th>
          <th>...</th>
          <th>...</th>
          ...
          <th>...</th>
        </tr>
    </thead>
    <tbody>
        <tr class="playerData">
            <td>...</td>
            <td>...</td>
            <td>...</td>
            <td>...</td>
            ...
            <td>...</td>
        </tr>
        ...
    </tbody>
</table>

The filters that users are given are as follows:

The javascript/jquery I have written and is likely the culprit is as follows:

function autoRank() {
    // auto number
    rank = 0;
    $("#myTablePlayers .playerData").each(function() {
        if ($(this).css("display") != "none") {
            rank++;
            $(this).find('td').eq(colRank).text(rank);
        }
    });
}

function filterTable() {
    // Need some error checking on input not number
    minGP = $("#mingp").val()
    teams = $("#teamFilter").val()
    position = $("#position").val()
    age = $("#age").val()

    $("#myTablePlayers .playerData").show();

    $("#myTablePlayers .playerData").each(function() {
        toHide = false;

        if (teams != "") {
            if ( !$(this).find('td').eq(colTeam).text().toUpperCase().includes(teams.toUpperCase())) {
                toHide = true;
            }
        }

        if ( Number($(this).find('td').eq(colGP).text()) < minGP ) {
            toHide = true;
        }

        if (position != "") {
            if (position == "D") {
                if ($(this).find('td').eq(colPos).text().indexOf("D") == -1) {
                    toHide = true;
                }
            } else if (position == "F") {
                if ($(this).find('td').eq(colPos).text().indexOf("D") != -1) {
                    toHide = true;
                }
            } else if ( $(this).find('td').eq(colPos).text() != position) {
                toHide = true;
            }
        }

        if (age != "") {
            column = Number($(this).find('td').eq(colAge).text())
            age = Number(age)
            if (  column < age || column >= age+1  ) {
                toHide = true;
            }
        }

        if (toHide == true) {
            $(this).hide();
        }

    });

    autoRank();
}

$("#teamFilter").on('change', filterTable);

$("#mingp").on('change', filterTable);

$("#position").on('change', filterTable);

$("#age").on('change', filterTable);

What is the inefficiency in this code that makes the browser hang? What should I be changing to make it efficient?

I looked at Google but jquery table filter plug ins do not give me the ability to filter rows based on specific columns based on specific inputs as outlined above (e.g. https://www.sitepoint.com/12-amazing-jquery-tables/).

Upvotes: 2

Views: 1003

Answers (2)

cFreed
cFreed

Reputation: 4484

Currently your code works like this:

  • iterate all rows
  • then for each row:
    • successively for each not-empty filter, look for all its child columns
    • then isolate the involved column and get its value

Only regarding the above exposed mechanism and using some numbers you cited it means that, with a unique simple filter like "teams" you actually touch 40000 columns (1000 rows * 1 filter * 40 columns).
But if 2 filters are not-empty it immediately grows to 80000 columns touched, and so on.

This is obviously a first realm where to find a way to work faster, with a pretty simple change like this:

  • iterate all rows
  • then for each row:
    • look for all its child columns
    • successively for each not-empty filter, then isolate the involved column and get its value

The involved part of code becomes:

$("#myTablePlayers .playerData").each(function() {
    var toHide = false,
        columns = $(this).find('td');

    if (teams != "") {
        if ( !columns.eq(colTeam).text().toUpperCase().includes(teams.toUpperCase())) {
            toHide = true;
        }
    }
    // ...same for next filters

This way, we already get rid of multiplying the column touches by the number of not-empty filters.

But we can go further...
In the current situation, each execution actually touches all the cells of the table, while at most 4 columns are involved (for the 4 filters). So we might try to find a way to reduce the total number of touched columns from 40000 to 4000!

This can be achieved by affecting a distinguishing class (say the filter-name) to these involved columns, so we might change the code like this: 

$("#myTablePlayers .playerData").each(function() {
    var toHide = false,
        classTeam = '.colTeam',
        classGP = `.colGP`,
        classPos = `.colPos`,
        classAge = `.colAge`;

    if (teams != "") {
        if ( !$(classTeam, this).text().toUpperCase().includes(teams.toUpperCase())) {
            toHide = true;
        }
    }
    // ...same for next filters

Maybe there is an issue with this:

It is generated from Python/Flask as a static HTML table

which means you don't have hand on the generated table.
If so, you can merely add the following to affect the class names just after the page load:

$(document).ready(function() {
    $("#myTablePlayers .playerData").each(function() {
        $('td', this).eq(colTeam).addClass(classTeam);
        $('td', this).eq(colGP).addClass(classGP);
        // ...
    }
}

But actually it might be improved another way (then the previous suggestion becomes useless), using a totally different method.
Since the table is static, we can act just after the page load (so only once) to prepare the needed data for a more direct access when filtering happens.

We can pre-register all the involved columns for each filter:

$(document).ready(function() {
    var teamCols = $(),
        GPCols = $(),
        posCols = $(),
        ageCols = $();
    $("#myTablePlayers .playerData").each(function() {
        var columns = $(this).find('td');
        teamCols.add(columns.eq(colTeam));
        GPCols.add(columns.eq(colGP));
        posCols.add(columns.eq(colPos));
        ageCols.add(columns.eq(colAge));
    }
}

Then the filter can process directly addressing the involved columns. BTW we can also immediately hide their parent (this was already possible in the original version):

if (teams) {
    teamCols.each(function() {
        if (!this.innerHTML.toUpperCase().includes(teams.toUpperCase())) {
            $(this).parent().hide();
        }
    }
}

This is written a bit quickly, so it might contain some flaws, and also might be yet improved...

Upvotes: 3

Mikey
Mikey

Reputation: 6766

You can improve your code by caching your re-used elements, and doing less iterations. Currently, you do three iterations over the records that can be condensed into one.

Here's how I would do it (untested):

$(function () {
    // cache the elements that you will be re-using
    var $minGP = $("#mingp"), 
        $team = $("#teamFilter"), 
        $position = $("#position"),
        $age = $("#age");

    var $rows = $("#myTablePlayers .playerData");

    function filterTable() {
        // Need some error checking on input not number
        var minGP = $minGP.val(),
            team = $team.val(),
            position = $position.val(),
            age = $age.val();

        // set the rank as we loop (instead of having a separate iteration for ranking)
        var rank = 0;

        // when you use show() and .each(), you are iterating over all the rows twice 

        // instead loop once
        $rows.each(function() {
            // cache current row as it will be re-used
            var $row = $(this);
            // set flag to show or hide
            var display = true;

            if (teams != "" && 
                !$row.find('td').eq(colTeam).text().toUpperCase().indexOf(teams.toUpperCase()) > -1) {
                display = false;
            }
            if (Number($row.find('td').eq(colGP).text()) < minGP) {
                display = false;
            }
            if (position != "") {
                if (position == "D" && 
                    $row.find('td').eq(colPos).text().indexOf("D") == -1) {
                    display = false;
                } else if (position == "F" && 
                    $row.find('td').eq(colPos).text().indexOf("D") != -1) {
                    display = false;
                } else if ($row.find('td').eq(colPos).text() != position) {
                    display = false;
                }
            }
            if (age != "") {
                column = Number($row.find('td').eq(colAge).text())
                age = Number(age)
                if (column < age || column >= age + 1) {
                    display = false;
                }
            }
            // hide or show row
            $tr.toggle(display);
            // set rank number
            if (display) {
                $row.find('td').eq(colRank).text(rank++);
            }
        });
    }
    // attach change event handler
    $minGP.add($team).add($position).add($age).change(filterTable);
});

This might speed up your code by few seconds, but ultimately it depends on how much and how big your data is.

Upvotes: 1

Related Questions