-3

I just noticed something very awkward I tested both methods RemoveAt() and Remove(), but they don't work the same and I am wondering if someone can exmplain the reason why. Basically the only method that successfully removes a specific item from the list in this example is Remove().

List<int> testList = new List<int>{1,76,3,4,5,76,76,8};

public void RemoveElements()
{
    for (int i = 0; i < testList.Count; i++)
    {
        //WORKS
        testList.Remove(76); //A 

        // DOES NOT WORK
        //if(testList[i] == 76) testList.Remove(testList[i]); //B 

        // DOES NOT WORK EITHER
        // if(testList[i] == 76) testList.RemoveAt(i); //C
    }
}

The output of A is:1,3,4,5,8 The output of B is:1,3,4,5,76,8 The output of C is:1,3,4,5,76,8

Thank you for your time!

Daniel
  • 9,491
  • 12
  • 50
  • 66
Nicholas
  • 51
  • 6

4 Answers4

2

Removing an item by index changes the indexes of items that come after it. This can lead to skipping some items. If you want to use RemoveAt while iterating through the list try iterating in reverse.

Mike
  • 435
  • 2
  • 7
1

The Remove and RemoveAt functions are actually working properly in your code. However using RemoveAt is the better function to use for efficiency if you are looping through the list because Remove has to search through the list again to find the first case where 76 occurs, while RemoveAt already knows the exact index to remove the item at.

The problem is occuring because of the way you are looping through the list.

As you iterate through and remove items from the list, your list shifts left by one index to fill in the item that was just removed while your i index is still at the same place (skipping the next item).
Usually the best practice is to iterate through a list backwards if you are removing elements from the list, because the end point is then dependent on it reaching 0 and handles the list shifting left each time which won't skip the next element.

List<int> testList = new List<int>{1,76,3,4,5,76,76,8};

public void RemoveElements()
{
    for (int i = testList.Count - 1; i >= 0; i--)
    {
        // RemoveAt option
        if(testList[i] == 76) testList.RemoveAt(i);
    }
}
Karl
  • 1,664
  • 2
  • 12
  • 19
1

Here's what your list contains to start with:

i:             0   1   2   3   4   5   6   7
testList[i]:   1  76   3   4   5  76  76   8

Now consider what happens during the loop iteration when i == 5, which means that testList[i] is the first of your two adjacent 76es. Running the code in version B or C will remove element 5 from testList, causing the elements after it to shift downward by one index. So testList[6], which was originally 76, is now 8; and testList[7], which was originally 8, is now past the end of your list. The next time the loop runs, it will be with i == 6, which means your code will consider the value 8, not the second of your 76es. That second 76 has been skipped entirely.

This is the reason behind hatchet's comment on your question saying that if you must iterate through the list, you should do so in reverse. If you're iterating backwards, then it doesn't matter that removing an element shifts the positions of elements further down the list, because you've already processed those. Of course a better approach still is to use a solution that does not require iteration at all, such as List<T>.RemoveAll.

Joe Farrell
  • 3,502
  • 1
  • 15
  • 25
0

Remove removes from collection the element equals to element you specified. RemoveAt removes element at specified position.

fdafadf
  • 809
  • 5
  • 13