1

I have Javascript Filter Item Class and array which is like below 

function FilterItem(filterId, filterType) {
    this.filterId = filterId;
    this.filterType = filterType;
}

var filterItemArray = [];

I am adding Filter Items to this array like below

function AddFilterItem(filterId, filterType) {
    filterItemArray.push(new FilterItem(filterId, filterType));
}

I also need to remove item from this array by specific filterId

function RemoveFilterItem(filterId, filterType) {
    var item = new filterItem(filterId, filterType);
    var itemIndex = jQuery.inArray(item, filterItemArray);
}

But this does not work and I dont think it is the efficient way? My Question is what is the best way to remove this item in RemoveFilterItem Method

Improve this question
8
  • 2
    You could search for it in the array and remove it. In a loop you can examine each element until you find it. What you have now will create another element with the same properties. Commented May 22, 2015 at 12:20
  • @Rudy yes this is my curreny approach right now, I just want to get item index by duplicated item in this array. I think I should create my own findItemIndex method, this will check each item till found then return item index if exist Commented May 22, 2015 at 12:22
  • You are creating a new instance, how do you think it would match a different instance? Commented May 22, 2015 at 12:28
  • 2
    Don't create global methods AddFilterItem and RemoveFilterItem, use FilterItem.prototype.add and FilterItem.prototype.remove Commented May 22, 2015 at 12:32
  • So...if this is a big, performance-sensitive collection, and you're often going to be finding elements in it via an ID...would it not make sense to turn it into a map (Or, in JavaScript terms, an Object)? That wouldn't prevent you from looping through the object keys when you just want to do something to each element. Commented May 22, 2015 at 13:23

3 Answers 3

Reset to default
3

Why don't you use the native filter function:

function RemoveFilterItem(filterId, filterType) {    
    filterItemArray = filterItemArray.filter(function (el) {
        return el.filterId !== filterId && el.filterType !== filterType;
    });
}

This will give you the array without that element with that id and type.

DEMO

And here's a modified version of Manwal's answer but without the jQuery again:

function RemoveFilterItem(filterId, filterType) {
    for (var i = 0, l = filterItemArray.length; i < l; i++) {
        var el = filterItemArray[i];
        if (el.filterId === filterId && el.filterType === filterType) {
            filterItemArray.splice(i, 1);

            // because we're caching the length of the array
            // we need to adjust the length of l once the splice has taken place
            l--;
        }
    }
}

DEMO

Improve this answer
Sign up to request clarification or add additional context in comments.

5 Comments

Upvoted, it's just not very elegant since you have to create a whole new array.
Doesn't use jQuery tho :)
@Andy That's definitely a plus, I am working on a jsperf to compare performance of using splice and searching instead
Add your result as a comment here as I'd like to see that.
@Andy Splice is waaaaay faster jsperf.com/filter-to-remove-items-from-array :) See my answer
2

Andy's answer works, but you can make it a lot faster using Array.splice. Manwal's answer uses splice but fails if there are duplicate filters.

enter image description here

I would also use OO code instead of global functions.

function FilterItem(filterId, filterType) {
    this.filterId = filterId;
    this.filterType = filterType;
}

function FilterItems() {
    this.items = [];
}

FilterItems.prototype.add = function (filterId, filterType) {
    this.items.push(new FilterItem(filterId, filterType));
}

FilterItems.prototype.remove = function (filterId, filterType) {
    for (var i = this.items.length - 1; i >=0 ; i--) {
        var item = this.items[i];
        if (item.filterId === filterId && item.filterType === filterType) {
            this.items.splice(i, 1);
        }    
    }
}

var filters = new FilterItems();
filters.add(1, 1);
filters.add(2, 2);
// Adding two filters of the same type/id to prove that it can remove
// multiple items
filters.add(1, 1);

filters.remove(1, 1);

console.log(filters.items.length);
console.log(filters.items[0]);

Improve this answer

1 Comment

Both your's and above Andy's answer works but I think this is the best approach for performance case, I accepted this as an answer
1

You can use $.each to iterate each element of array and then splice it:

function RemoveFilterItem(filterId, filterType) {    
    var item = new FilterItem(filterId, filterType);
    $.each(filterItemArray, function( index, value){
        if(value.filterId == item.filterId){
            filterItemArray.splice(index,1);
        }
    });   
}

See it in action

No need to create new FilterItem while removing:

function RemoveFilterItem(filterId, filterType) {    
    $.each(filterItemArray, function( index, value){
        if(value.filterId === filterId && value.filterType === filterType){
            filterItemArray.splice(index,1);
        }
    });   
}

See Updated Demo

Improve this answer

5 Comments

Why are you creating a new FilterItem just to delete it?
@JuanMendes added modified solution also.
This will fail if there are two items with the same id/type because you're modifying the array and the index is shifting, you can fix that by looping backwards.
I can understand this will fail on this situation. if i shall use $.each(filterItemArray.reverse(), function(i, v) this condition can happen also. What do you think?
No it wouldn't, you have to loop backwards, not just reverse the array, because if you still loop forward, it will shift the index of the items you haven't seen yet

Your Answer

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.