2
\$\begingroup\$

Is there a way to make this smarter?

Here is the complete code jsbin link

the code needs to filter the table, and show in the parameters list, only the filters that remain in the table, and with the number aside the parameter.

<div class="filterColumn">
<h2>size</h2>
    <span class="filter" data-filterby="size" data-count="0" data-val="1">1(1)</span>
    <span class="filter" data-filterby="size" data-count="0" data-val="2">2(3)</span>
    <span class="filter" data-filterby="size" data-count="0" data-val="3">3(4)</span>
    <span class="filter" data-filterby="size" data-count="0" data-val="4">4(2)</span>
</div>
<div class="filterColumn">
<h2>height</h2>
    <span class="filter" data-filterby="height" data-count="0" data-val="7">7(3)</span>
    <span class="filter" data-filterby="height" data-count="0" data-val="8">8(3)</span>
    <span class="filter" data-filterby="height" data-count="0" data-val="11">11(4)</span>
</div>
<div class="filterColumn">
<h2>weight</h2>
    <span class="filter" data-filterby="weight" data-count="0" data-val="22">22(2)</span>
    <span class="filter" data-filterby="weight" data-count="0" data-val="45">45(5)</span>
    <span class="filter" data-filterby="weight" data-count="0" data-val="88">88(1)</span>
    <span class="filter" data-filterby="weight" data-count="0" data-val="90">90(1)</span>
    <span class="filter" data-filterby="weight" data-count="0" data-val="11">11(1)</span>
</div>
<div class="filterColumn">
<h2>color</h2>
    <span class="filter" data-filterby="color" data-count="0" data-val="red">red(2)</span>
    <span class="filter" data-filterby="color" data-count="0" data-val="green">green(2)</span>
    <span class="filter" data-filterby="color" data-count="0" data-val="blue">blue(2)</span>
    <span class="filter" data-filterby="color" data-count="0" data-val="grey">grey(2)</span>
    <span class="filter" data-filterby="color" data-count="0" data-val="black">black(1)</span>
    <span class="filter" data-filterby="color" data-count="0" data-val="white">white(1)</span>
</div>
<table id="productTable">
    <tr>
        <th>Model</th>
        <th>size</th>
        <th>height</th>
        <th>weight</th>
        <th>color</th>
    </tr> 
    <tr data-size="1" data-height="7" data-weight="22" data-color="red">
         <td>111az</td>
         <td>1</td>
         <td>7</td>
         <td>22</td>
         <td>red</td>
     </tr> 
     <tr data-size="2" data-height="7" data-weight="45" data-color="red">
         <td>111as</td>
         <td>2</td>
         <td>7</td>
         <td>45</td>
         <td>red</td>
     </tr> 
     <tr data-size="2" data-height="7" data-weight="45" data-color="green">
         <td>111af</td>
         <td>2</td>
         <td>7</td>
         <td>45</td>
         <td>green</td>
     </tr> 
     <tr data-size="2" data-height="8" data-weight="45" data-color="green">
         <td>111ag</td>
         <td>2</td>
         <td>8</td>
         <td>45</td>
         <td>green</td>
     </tr> 
     <tr data-size="3" data-height="8" data-weight="45" data-color="blue">
         <td>111ah</td>
         <td>3</td>
         <td>8</td>
         <td>45</td>
         <td>blue</td>
     </tr> 
     <tr data-size="3" data-height="8" data-weight="45" data-color="blue">
         <td>111aj</td>
         <td>3</td>
         <td>8</td>
         <td>45</td>
         <td>blue</td>
     </tr> 
     <tr data-size="3" data-height="11" data-weight="22" data-color="grey">
         <td>111az</td>
         <td>3</td>
         <td>11</td>
         <td>22</td>
         <td>grey</td>
     </tr> 
     <tr data-size="3" data-height="11" data-weight="88" data-color="grey">
         <td>111az</td>
         <td>3</td>
         <td>11</td>
         <td>88</td>
         <td>grey</td>
     </tr> 
     <tr data-size="4" data-height="11" data-weight="90" data-color="black">
         <td>111ak</td>
         <td>4</td>
         <td>11</td>
         <td>90</td>
         <td>black</td>
     </tr> 
     <tr data-size="4" data-height="11" data-weight="11" data-color="white">         
         <td>111al</td>
         <td>4</td>
         <td>11</td>
         <td>11</td>
         <td>white</td>
     </tr> 
</table>
<div id="reset">reset</div>

the jQuery code

var thisfiltertitle = ["size","height","weight","color"];

$("#reset").click(function() {
    $("#productTable tr:hidden").show();
    $(".filterColumn").find("span").show();
    UpdateData();  
});

$(".filter").click(function() {
    var filterby = $(this).data("filterby").trim();
var val = $(this).data("val");
    var rows = $("#productTable tr:visible:gt(0)");
rows.each(function(index) {
    var row = $(this);
            if ($(this).data(filterby) == val) {
                row.show();
            }else{
                row.hide();
            }
    });
    UpdateData();
});

function UpdateData() {
    $(".filterColumn").find("span").each(function(index) {
        $(this).attr("data-count",0);
        $(this).html($(this).attr("data-val") + "(0)");
    });
    for(i=0; i<thisfiltertitle.length; i++) {
        $("tr[data-" + thisfiltertitle[i] + "]:visible").each(function(index) {
            thisoption = $("span[data-filterby=" + thisfiltertitle[i] + "][data-val='" + $(this).data(thisfiltertitle[i]) + "']");
            thisoption.attr("data-count",parseInt(thisoption.attr("data-count"),10)+1);
        });
    }

    $(".filterColumn").find("span[data-count=0]").hide();
    $(".filterColumn").find("span").each(function(index) {
         $(this).html($(this).attr("data-val") + "(" + $(this).attr("data-count") + ")");
    });
}
\$\endgroup\$
  • 1
    \$\begingroup\$ Already quite smarter than a lot of JS I see these days, if you ask me :) A lead for improvement would be to separate more clearly presentation from business logic (too much data in data-attributes) : refactor this to hold all data in an array of objects, perform searches on this array instead of the DOM, and create & update filters / table dynamicly using this array. Apart of that, such code would be a perfect candidate for an AngularJS Directive \$\endgroup\$ – m_x Nov 22 '13 at 11:48
  • 1
    \$\begingroup\$ ps: using an architecture based on an array will make your life easier if you want to add AJAX actions later. pps: you might also want to use a js templating system for this \$\endgroup\$ – m_x Nov 22 '13 at 11:50
  • \$\begingroup\$ what approach are you taking to render this? ie. are you generating your initial filter elements through some server side logic, or would you be happy to look at solutions which involve creating these elements on the fly, clientside, based on a viewmodel? \$\endgroup\$ – Stephen James Nov 28 '13 at 12:08
1
\$\begingroup\$

Overall I think it works pretty well. There are just a few minor issues with the existing implementation that I could highlight :

Ensuring State "Safety"

On first render filter items have a data-count value of 0, which only gets updated after the first call to UpdateData, nevertheless on first render they are technically in an incorrect state.

data-val is set correctly (hard-coded) in the code though on first render, but also gets updated with calls to UpdateData. The concern here is there are two independant things which are responsible for the rendering of these attributes (the hard-coded or server rendering of the .filter elements, and the script) which leaves room for error. By making sure only one thing is responsible for the state of those attributes will make sure they don't get muddled.

I would opt to not hard-code or render the data- attributes on the server, but rather to generate them on the clientside only and so to make sure that UpdateData() is called when the page loads.

One directional

Because this process only scans the rows that are visible (by using the tr[data-" + thisfiltertitle[i] + "]:visible selector) the outcome is that you can't ever undo a filtered value without resetting the entire filter.

If you ever wanted to add your filter options as checkboxes, not spans, and allow for filtered options to be undone, you'd have to change things up a bit to get around this.

Repeated Selectors

This is only relevant if you're fine combing through your performance, but its worth noting that you could improve the code a little bit by storing the results of jquery selectors that you use more than once in variables for more efficient re-use.

Also to minimise the number of event handlers registered, you could use jQuery's delegate on the container of the span.filter elements, rather than having click events on each element.

Client-Side Templating

*m_x mentioned this in their comment, but just to expand on this a little*

Your example contains a fair amount of repeated content, ie state is maintained both in data-* attributes and naturally in the display of element text values. Absolutely nothing wrong with this, but it does make things a bit tricky to modify this state. ie. When you want to pull that state out quickly to work with it or look at it, you have to loop through the dom elements and grab their data attributes and build up that model each time.

If you used some client side templating, or something like KnockoutJS, you would just have to update the model (an array of objects in memory) and the HTML on the page would update in response to the changes, due to the publish-subscribe nature of the template (and in the case of KnockoutJS, observables).

Hope that helps, your code's smart code already, but wanted to mention the few things I could find or suggest!

\$\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.