How to remove elements from an array

问题:

Hi I'm working on some legacy code that goes something along the lines of

for(int i = results.Count-1; i >= 0; i--)
{
  if(someCondition)
  {
     results.Remove(results[i]);
  }
}

To me it seems like bad practice to be removing the elements while still iterating through the loop because you'll be modifying the indexes.

Is this a correct assumption?

Is there a better way of doing this? I would like to use LINQ but I'm in 2.0 Framework

回答1:

The removal is actually ok since you are going downwards to zero, only the indexes that you already passed will be modified. This code actually would break for another reason: It starts with results.Count, but should start at results.Count -1 since array indexes start at 0.

for(int i = results.Count-1; i >= 0; i--)
{
  if(someCondition)
  {
     results.RemoveAt(i);
  }
}

Edit:

As was pointed out - you actually must be dealing with a List of some sort in your pseudo-code. In this case they are conceptually the same (since Lists use an Array internally) but if you use an array you have a Length property (instead of a Count property) and you can not add or remove items.

Using a list the solution above is certainly concise but might not be easy to understand for someone that has to maintain the code (i.e. especially iterating through the list backwards) - an alternative solution could be to first identify the items to remove, then in a second pass removing those items.

Just substitute MyType with the actual type you are dealing with:

List<MyType> removeItems = new List<MyType>();

foreach(MyType item in results)
{
   if(someCondition)
   {
        removeItems.Add(item);
   }
}

foreach (MyType item in removeItems)
    results.Remove(item);

回答2:

It doesn't seem like the Remove should work at all. The IList implementation should fail if we're dealing with a fixed-size array, see here.

That being said, if you're dealing with a resizable list (e.g. List<T>), why call Remove instead of RemoveAt? Since you're already navigating the indices in reverse, you don't need to "re-find" the item.

回答3:

May I suggest a somewhat more functional alternative to your current code:

Instead of modifying the existing array one item at a time, you could derive a new one from it and then replace the whole array as an "atomic" operation once you're done:

The easy way (no LINQ, but very similar):

Predicate<T> filter = delegate(T item) { return !someCondition; };

results = Array.FindAll(results, filter);
// with LINQ, you'd have written: results = results.Where(filter);

where T is the type of the items in your results array.

---

A somewhat more explicit alternative:

var newResults = new List<T>();
foreach (T item in results)
{
    if (!someCondition)
    {
        newResults.Add(item);
    }
}
results = newResults.ToArray();

回答4:

Usually you wouldn't remove elements as such, you would create a new array from the old without the unwanted elements.

If you do go the route of removing elements from an array/list your loop should count down rather than up. (as yours does)

回答5:

a couple of options:

List<int> indexesToRemove = new List<int>();
for(int i = results.Count; i >= 0; i--)
{
  if(someCondition)
  {
     //results.Remove(results[i]);
       indexesToRemove.Add(i);
  }
}

foreach(int i in indexesToRemove) {
results.Remove(results[i]);
}

or alternatively, you could make a copy of the existing list, and instead remove from the original list.

//temp is a copy of results
for(int i = temp.Count-1; i >= 0; i--)
{
  if(someCondition)
  {
     results.Remove(results[i]);
  }
}