Remove from ObservableCollection items which name occurs on other list

Question

I've got ObservableCollection a and List b

Now I want to remove from collection a elements that have their equivalent in list b.

My code at this moment:

public static void CrossRemove<TFirst, TSecond>(this ObservableCollection<TFirst> collection, IEnumerable<TSecond> secondCollection, Func<TFirst, TSecond, bool> predicate)
{
    collection.Where(first => secondCollection.Any(second => predicate(first, second)))
        .ToList().ForEach(item => collection.Remove(item));
}

usage:

ObservableCollection<string> first = new ObservableCollection<string> { "1", "2", "3", "4", "5", "6", "k" };

IEnumerable<int> second = new List<int> { 2, 3, 5 };

first.CrossRemove(second, (x, y) => x == y.ToString());

this code removes "2", "3" and "5" from collection leaving "1", "4", "6" and "k".

In my real code a and b contains elements that inherits from the same interface and I'm comparing property that is in that interface but I'm not able to take any adventage of it.

I can't make a new list because it's bound to wpf view and there will be visible glitches if I do that instead of removing items.

Is there any better / faster way of doing this?

Answer 1

You can make your second collection a HashSet<T> for faster lookups. I also changed your ForEach to a foreach. This is easier to demonstrate with properties, as in your original.

void Main()
{
    ObservableCollection<MyClass> first = new ObservableCollection<MyClass> { "1", "2", "3", "4", "5", "6", "k" };

    ISet<IMyInterface> second = new HashSet<IMyInterface>(new MyClass2[] { 2, 3, 5 }, new MyEqualityComparer());

    first.CrossRemove(second);

    Console.WriteLine(string.Join(", ", first.Select(x => x.MyProperty)));
    // 1, 4, 6, k
}
public interface IMyInterface
{
    string MyProperty { get; set; }
}
public class MyEqualityComparer : IEqualityComparer<IMyInterface>
{
    public bool Equals(IMyInterface a, IMyInterface b)
    {
        return a.MyProperty == b.MyProperty;
    }
    public int GetHashCode(IMyInterface obj)
    {
        return obj.MyProperty.GetHashCode();
    }
}
public static class Extensions
{
    public static void CrossRemove<TFirst, TSecond>(this ObservableCollection<TFirst> collection, ISet<TSecond> set) where TFirst : TSecond
    {
        foreach (var item in collection.Where(item => set.Contains(item)).ToList())
            collection.Remove(item);
    }
}
public class MyClass : IMyInterface
{
    public string MyProperty { get; set; }
    public static implicit operator MyClass(string s)
    {
        return new MyClass { MyProperty = s };
    }
}
public class MyClass2 : IMyInterface
{
    public string MyProperty { get; set; }
    public static implicit operator MyClass2(int i)
    {
        return new MyClass2 { MyProperty = i.ToString() };
    }
}

Even if the object don't share a common interface, you should be able to write an IEqualityComparer<object> that works with both correctly, e.g. if your lambda predicate would've been:

(TypeA a, TypeB b) => a.PropA == b.PropB

Then your class would be:

public class MyOtherEqualityComparer : IEqualityComparer<object>
{
    private object GetProperty(object obj)
    {
        if (obj is TypeA)
            return ((TypeA)obj).PropA;
        else if (obj is TypeB)
            return ((TypeB)obj).PropB;
        else
            throw new Exception();
    }
    public bool Equals(object a, object b)
    {
        return GetProperty(a).Equals(GetProperty(b));
    }
    public int GetHashCode(object obj)
    {
        return GetProperty(obj).GetHashCode();
    }
}

Answer 2

I think that the simplest way to do this is with an equivalent of List<T>'s RemoveAll function as it is more general. Instead of

first.CrossRemove(second, (x, y) => x == y.ToString());

I would write

first.RemoveAll(item1 => second.Any(item2 => item1 == item2.ToString()));

Unfortunately, ObservableCollection<T> does not have this method, so we need to write one:

public static class Extensions
{
    public static void RemoveAll<T>(this ICollection<T> collection, Func<T, bool> pred)
    {
        var toBeRemoved = collection.Where(pred).ToArray();
        foreach (var item in toBeRemoved)
            collection.Remove(item);
    }
}

EDIT:

The above extension method is very inefficient, others such as this one are algorithmically much faster. In this case though, I don't think that it's relevant as we're talking about an ObservableCollection<T> that is presumably bound to a view. Given this, we should only be doing a very small number of changes, otherwise the layout and re-rendering costs will be very high. If you are doing a large number of changes, then you should probably be replacing the collection with a new one so that the layout is only recalculated once.