4
\$\begingroup\$

I'm building a list of filters for a website user to filter on, such as a product category page. I'm giving the user the option to filter on brandname, model name, or gender.

To avoid serverside code I'm trying to build it clientside. So, I'm looking on the different data-attr and building an array of filtering values of what's actually available to filter on.

I have made it work, but I'm not sure it's the most efficient way to do it.

HTML:

<div id="container">
    <div class="mix Brand_1 Model_2 Gender_0" data-Gender="0" data-Brand="Casio" data-Model=""></div>
    <div class="mix Brand_3 Model_1 Gender_0" data-Gender="0" data-Brand="Casio" data-Model=""></div>
    <div class="mix Brand_4 Model_1 Gender_2" data-Gender="2" data-Brand="Seiko" data-Model="Precision"></div>
    <div class="mix Brand_4 Model_1 Gender_1" data-Gender="1" data-Brand="Nikon" data-Model="Lada"></div>
    <div class="mix Brand_5 Model_2 Gender_1" data-Gender="1" data-Brand="DELL" data-Model="Inspirion"></div>
</div>

JS:

// Loop through all div's with .mix class 

var Genders = [];
var Brands =  [];
var Models = []; 
$( ".mix" ).each(function() {
   // build arrary with Gender

   if ($(this).attr('data-Gender')) {
           genderFilter = $(this).attr('data-Gender');
            var found = jQuery.inArray(genderFilter, Genders);
       if (found >= 0) { } else {
                // Element was not found, add it.
                Genders.push(genderFilter);
            }
   }
   // build arrary with Models
    if ($(this).attr('data-Brand')) {
           brandFilter = $(this).attr('data-Brand');
            var found = jQuery.inArray(brandFilter, Brands);
       if (found >= 0) { } else {
                // Element was not found, add it.
                Brands.push(brandFilter);
            }
   }
   // build arrary with Brands
    if ($(this).attr('data-Model')) {
           modelFilter = $(this).attr('data-Model');
            var found = jQuery.inArray(modelFilter, Models);
       if (found >= 0) { } else {
                // Element was not found, add it.
                Models.push(modelFilter);
            }
   }
});

console.log( Models.length );
console.log ( Models);
console.log( Brands.length );
console.log ( Brands);
console.log( Genders.length );
console.log ( Genders);

JSFiddle

\$\endgroup\$
1
  • 1
    \$\begingroup\$ Welcome to Code Review! You passed through the "First Post" queue with flying colors! \$\endgroup\$
    – RubberDuck
    Commented Oct 11, 2014 at 19:22

1 Answer 1

1
\$\begingroup\$

It is customary to use lowercase names for variables and Capitalized names for classes, so I would rename Genders -> genders, and likewise the Models and Brands arrays too.


The if blocks are a bit tediously written. They would be better this way:

var genderFilter = $(this).attr('data-Gender');
if (genderFilter) {
    if (jQuery.inArray(genderFilter, genders) == -1) {
         // Element was not found, add it.
         genders.push(genderFilter);
    }
}

Btw, note that when you retrieve data-Gender="0" using .attr('data-Gender'), it's treated as the string "0", and in if ("0") { ... } it evaluates to true, so the genders array will have the values 0, 2, 1. Perhaps you might want to use .data('genders') instead, in which case the genders array will have the values 2, 1.

For models and brands, using .data('model') instead of .attr('data-Model') will give the same result, so I suggest using the simpler.


For genders, models, brands, you do essentially the same thing. This calls for a helper function:

function addToArrayIfNew(arr, item) {
    if (item && jQuery.inArray(item, arr) == -1) {
        arr.push(item);
    }
}

With this, you could simplify the entire code like this:

$( ".mix" ).each(function() {
    addToArrayIfNew(genders, $(this).attr('data-Gender'));
    addToArrayIfNew(models, $(this).data('model'));
    addToArrayIfNew(brands, $(this).data('brand'));
});
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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