Hiding Rows Given A Certain Criteria (slow)

Question

I'm trying to hide tr's within a html table if the inputs inside them match a certain criteria. The criteria is defined by a dropdown's selected value. I'm doing it like so:

$(function () {

  $('body').find('#p_Selection').live('change', function () {

    var type = $('body').find('#p_Selection').attr('value');
    var tableRow = $('.goods').find('.detail-child tr');

    tableRow.each(function (index) {

      var Record_LidExpected = $('input[id$=Record[' + index + ']_LidExpected]').attr('value');
      var Record_LidObtained = $('input[id$=Record[' + index + ']_LidObtained]').attr('value');
      var Record_QuantityExpected = $('input[id$=Record[' + index + ']_QuantityExpected]').attr('value');
      var Record_QuantityObtained = $('input[id$=Record[' + index + ']_QuantityObtained]').attr('value');

      if (type == "1") {

        if (Record_LidExpected != Record_LidObtained) {
          $(this).hide();
        }
        else {
          if (Record_QuantityExpected != Record_QuantityObtained) {
            $(this).hide();
          }
        }
      }
      else {
        if (type == "2") {
          if (Record_LidExpected == Record_LidObtained) {
            $(this).hide();
          }
          else {
            if (Record_QuantityExpected == Record_QuantityObtained) {
              $(this).hide();
            }
          }
        }
        else {
          if (type == "0") {
            $(this).show();
          }
        }
      }
    });
  });
});​

This script became extremely slow inside my aspx page and it just won't complete because it is too heavy. Any suggestions on how to make it faster?

Answer 1

In my rewrite you'll notice a few things.

1. I ommitted $('body').find() because it doesn't give you any advantage over selecting #p_Selection directly.
2. When selecting the input values using the $() function, I am adding this as a second argument. This basically tells jQuery to look for the input within this (which in this case refers to the current tr in the each loop. The advantage here is that jQuery doesn't need to search the entire DOM for that particular input, just within the current tr tag.
3. I just cleaned up your if/else logic a bit using a switch statement and some || operators.

I think that this should run faster now.

<script type="text/javascript">
  $(function() {

    $('#p_Selection').live('change', function() {
      var type = $(this).val();

      var Record_LidExpected, 
          Record_LidObtained, 
          Record_QuantityExpected, 
          Record_QuantityObtained;

      $('.goods .detail-child tr').each(function(index) {

        Record_LidExpected = $('input[id$=Record['+index+']_LidExpected]', this).val();
        Record_LidObtained = $('input[id$=Record[' + index + ']_LidObtained]', this).val();
        Record_QuantityExpected = $('input[id$=Record[' + index + ']_QuantityExpected]', this).val();
        Record_QuantityObtained = $('input[id$=Record[' + index + ']_QuantityObtained]', this).val();

        switch(type) {
          case "1" :
            if (Record_LidExpected != Record_LidObtained || Record_QuantityExpected != Record_QuantityObtained) {
              $(this).hide();
            }
          break;
          case "2" :
            if (Record_LidExpected == Record_LidObtained || Record_QuantityExpected == Record_QuantityObtained) {
              $(this).hide();
            }
          break;

          case "0":
            $(this).show();
          break;
        }
      });
    });
  });
</script>

Answer 2

The key points of preformance optimization

• do few selects (don't select everything onchange)
• save selections that won't change
• be as specific as possible when you select elements

Bonus: learn to use else if because your branches become clearer.

Here is the code:

$(function () {

  // pre-select things that won't change
  var $sel = $('#p_Selection'); 
  var tableRow = $('.goods .detail-child tr');

  //
  // ONCHANGE EVENT 
  // keep in mind that everything in it will be very costly
  //
  $sel.live('change', function () {

    var type = $sel.attr('value');

    tableRow.each(function (index) {

      // this just makes the code shorter
      var record = '#Record'+index;

      // simple ID selector is enough, since its unique
      var Record_LidExpected =  $(record+'_LidExpected').attr('value');
      var Record_LidObtained =  $(record+'_LidObtained').attr('value');
      var Record_QuantityExpected = $(record+'_QuantityExpected').attr('value');
      var Record_QuantityObtained = $(record+'_QuantityObtained').attr('value');

      if (type == "1") {
        if (Record_LidExpected != Record_LidObtained) {
          $(this).hide();
        } else if (Record_QuantityExpected != Record_QuantityObtained) {
          $(this).hide();
        }
      } else if (type == "2") {
        if (Record_LidExpected == Record_LidObtained) {
          $(this).hide();
        } else if (Record_QuantityExpected == Record_QuantityObtained) {
          $(this).hide();
        }
      } else if (type == "0") {
        $(this).show();
      }
    });
  });
});