2

I have a few check boxes which filter products depending on what checkboxes have been selected. The filter works when the check boxes are clicked, and product ids are added to an array. As you can see both filters have some of the same ids, so if they click more than one input boxes I want to make sure the ids are only pushed into the array once. Eg. if they have input.white selected and input.greymelange selected then they clicked the filter-btn I only want repeated ids to be pushed once. Is there a way I can do this?

JQUERY

var productIds = [""];

$(document).on('click', 'a.filter-btn', function(e){

    $( ".listingTemplate" ).html("");
   if ($('input.white').is(':checked')) {
          productIds.push("4021401143741", "4021402266227");
    } 

   if ($('input.greymelange').is(':checked')) {
          productIds.push("4021401143741", "4021402266227","4021402266418", "4021401143796");
    } 
});

HTML

 <input type="checkbox" id="white" class="white" value="white" />                
 <input type="checkbox" id="greymelange" class="greymelange" value="greymelange" />
<a href="#" class="filter-btn">filter</a>
user1937021
  • 10,151
  • 22
  • 81
  • 143
  • ...What exactly are you trying to do with this code? I think redesigning your code to loop through the products rather than the filters would simplify your code and also be more database-friendly. – user1618143 Dec 09 '13 at 19:51

6 Answers6

4

You can make use $.inArray() and $.each()

1) Keep all your new values within an array deArray.
2) Iterate this array using $.each and then find whether it is in productIds using inArray()

if ($('input.greymelange').is(':checked')) {
        var deArray = ["4021401143741", "4021402266227", "4021402266418", "4021401143796"];
        $.each(deArray, function (i, j) {
            if ($.inArray(j, productIds) == -1) {
                productIds.push(j);
            }
        });
    }

Suggestion: don't declare empty string in array.

var productIds = [""];  //considered as an string. This will affect 
                        //when you check length, now it is 1
var productIds = []; //length is 0

Finally,

var productIds = [];

$(document).on('click', 'a.filter-btn', function (e) {
    $(".listingTemplate").html("");
    if ($('input.white').is(':checked')) {
        var dArray = ["4021401143741", "4021402266227"];
        $.each(dArray, function (i, j) {
            if ($.inArray(j, productIds) == -1) {
                console.log(j)
                productIds.push(j);
            }
        })
    }

        if ($('input.greymelange').is(':checked')) {
            var deArray = ["4021401143741", "4021402266227", "4021402266418", "4021401143796"];
            $.each(deArray, function (i, j) {
                if ($.inArray(j, productIds) == -1) {
                    productIds.push(j);
                }
            })
        }
            console.log(productIds)
        });

JSFiddle

Praveen
  • 55,303
  • 33
  • 133
  • 164
2

If you only have a fixed set of product ID's, why do you need a variable-length array to store which product ID's are checked?

I would suggest maybe building up an object which contains this logic that you can use.

function productMapper() {
    var colorToIdMap = {
        'white' = [
            '4021401143741',
            '4021402266227'
        ],
        'greymelange' = [
            '4021401143741',
            '4021402266227',
            '4021402266418',
            '4021401143796'
        ]
    };
    var selectedIds = [];
    function selectIds(color) {
        this.selectedIds = [];
        var colorIds = this.colorToIdMap[color];
        for(i = 0; i < colorIds.length; i++) {
           var currentId = colorIds[i];
           if(this.selectedIds.indexOf(currentId) === -1) {
               this.selectedIds.push(currentId);
           }
        }
    }
    function getSelectedIds() {
        return this.selectedIds;
    }
}

Usage might be:

var selectedProductMap = new productMapper();

$(document).on('click', 'a.filter-btn', function(e){

    $( ".listingTemplate" ).html("");
    if ($('input.white').is(':checked')) {
          selectedProductMap.selectIds('white');
    } 

    if ($('input.greymelange').is(':checked')) {
          selectedProductMap.selectIds('greymelange');
    } 
});

The benefit here is that when you add new colors and product Id's it is easy to just add them to this class (or have that portion of class built dynamically from say a database.

I would also suggest looking at refactoring the HTML such that you don't need to add an if condition for each color option. Perhaps give all the checkbox inputs a common class and derive color value from input property. Something like this could go in your onclick function:

$('input.filterCheckbox').each(function(index.value)) {
    var $currentCheckbox = $(this);
    if($currentCheckbox.is(':checked')) {
        selectedProductMap.selectIds($currentCheckbox.val());
    }
}

var currentlySelectedIds = selectedProductMap.getSelectedIds();
Mike Brant
  • 70,514
  • 10
  • 99
  • 103
0

You need to check if the item is contained before you add to the list. Unfortunately this isn't as easy as it should be but the following stack explains that: How do I check if an array includes an object in JavaScript?

Since you tagged this as jQuery, the following (example) code might work for you:

if ($.inArray("4021401143741", productIds) === -1)
   productIds.push("4021401143741");

For more information on this call see here. Good luck!

Community
  • 1
  • 1
drew_w
  • 10,320
  • 4
  • 28
  • 49
0

To check if an item already exists in an array, you can use Array.indexOf. Please note that is not present natively in IE < 9, but I have posted a code snipped below that will add it to the Array prototype if it isn't available.

var products = [];
function addProducts() {
    for(var i = 0, l = arguments.length; i < l; i++) {
        if(products.indexOf(arguments[i]) === -1) {
            products.push(arguments[i]);
        }
    }
}
//Add indexOf if it doesn't exist natively
if (!Array.prototype.indexOf) {
    Array.prototype.indexOf = function(elt) {
        var len = this.length >>> 0;
        var from = Number(arguments[1]) || 0;
        from = (from < 0)
             ? Math.ceil(from)
             : Math.floor(from);
        if (from < 0)
          from += len;

        for(; from < len; from++) {
          if(from in this && this[from] === elt)
            return from;
        }
        return -1;
    };
}

Then to use these new functions, you can modify your code to be:

$(document).on('click', 'a.filter-btn', function(e) {
    if($('input.white').is(':checked')) {
        addProducts("4021401143741", "4021402266227");
    }
    if($('input.greymelange').is(':checked')) {
        addProducts("4021401143741", "4021402266227","4021402266418", "4021401143796");
    }
});

Just as a side note. If you can avoid it, you shouldn't bind events to the document. Binding events to the document can cause performance issues if you use a lot of events. Every time a click event is triggered, this function will run to check if the target matches your specified target. To limit this performance hit, you should bind them to the closest non changing element. In the example of inputs inside of a form, you could add it to the form.

Kyle
  • 4,421
  • 22
  • 32
0

Add the below code into ischecked conditional. // Continue execution for pushing values.

isSet = false;
currentVal = '999999999999';
$.each(productIds, function( index, value ) {
  if(value == currentVal){
     isSet = true;
  }
});

if(isSet){
// Continue execution
} else {
// failure
}
rawr
  • 31
  • 2
0

Does it need to be an array? It sounds like you'd be better off using a plain object:

productIds[ 420… ] = true;

It depends upon the rest of your code, but it sounds like you want to implement a hash / lookup table / dictionary rather than an array — it would also be more straightforward to check for existence this way.

Barney
  • 16,181
  • 5
  • 62
  • 76