Best way to remove items from a collection

If RoleAssignments is a List<T> you can use the following code.

workSpace.RoleAssignments.RemoveAll(x =>x.Member.Name == shortName);

@smaclell asked why reverse iteration was more efficient in in a comment to @sambo99.

Sometimes it's more efficient. Consider you have a list of people, and you want to remove or filter all customers with a credit rating < 1000;

We have the following data

"Bob" 999
"Mary" 999
"Ted" 1000

If we were to iterate forward, we'd soon get into trouble

for( int idx = 0; idx < list.Count ; idx++ )
{
    if( list[idx].Rating < 1000 )
    {
        list.RemoveAt(idx); // whoops!
    }
}

At idx = 0 we remove Bob, which then shifts all remaining elements left. The next time through the loop idx = 1, but list[1] is now Ted instead of Mary. We end up skipping Mary by mistake. We could use a while loop, and we could introduce more variables.

Or, we just reverse iterate:

for (int idx = list.Count-1; idx >= 0; idx--)
{
    if (list[idx].Rating < 1000)
    {
        list.RemoveAt(idx);
    }
}

All the indexes to the left of the removed item stay the same, so you don't skip any items.

The same principle applies if you're given a list of indexes to remove from an array. In order to keep things straight you need to sort the list and then remove the items from highest index to lowest.

Now you can just use Linq and declare what you're doing in a straightforward manner.

list.RemoveAll(o => o.Rating < 1000);

---

For this case of removing a single item, it's no more efficient iterating forwards or backwards. You could also use Linq for this.

int removeIndex = list.FindIndex(o => o.Name == "Ted");
if( removeIndex != -1 )
{
    list.RemoveAt(removeIndex);
}

If it's an ICollection then you won't have a RemoveAll method. Here's an extension method that will do it:

    public static void RemoveAll<T>(this ICollection<T> source, 
                                    Func<T, bool> predicate)
    {
        if (source == null)
            throw new ArgumentNullException("source", "source is null.");

        if (predicate == null)
            throw new ArgumentNullException("predicate", "predicate is null.");

        source.Where(predicate).ToList().ForEach(e => source.Remove(e));
    }

Based on: http://phejndorf.wordpress.com/2011/03/09/a-removeall-extension-for-the-collection-class/

If you want to access members of the collection by one of their properties, you might consider using a Dictionary<T> or KeyedCollection<T> instead. This way you don't have to search for the item you're looking for.

Otherwise, you could at least do this:

foreach (SPRoleAssignment spAssignment in workspace.RoleAssignments)
{
    if (spAssignment.Member.Name == shortName)
    {
        workspace.RoleAssignments.Remove(spAssignment);
        break;
    }
}