correct way of looping through a list and remove items

Question

I wrote a function to go through a list and remove list items if some conditions where met. My program crashed on it, and after a while i concluded that the outer for loop, goes through all items in the list. While at the same routine the list of item can get shorter.

 // Lijst is a list of a struct that contains a value .scanned and .price

 for (int i = 0; i < Lijst.Count; i++)
  {
   if (Lijst[i].scanned == false)
    {
     // (removed deletion of list item i here)
     if (Lijst[i].price > (int)nudMinimum.Value)
      {
       Totaal++;
        lblDebug.Text = Totaal.ToString();
      }
    Lijst.RemoveAt(i); //<-moved to here
    } 
  }

Now i wonder whats the correct to do this, without getting index out of range errors.

Also note that `Lijst[i]` in the `if` accesses the element _after_ the one you just removed. — CompuChip, Jan 28 '16 at 09:45
While removing items from list, you want the loop to run backwards. — Harry, Jan 28 '16 at 09:46
Possible duplicate of [C# List - Removing items while looping / iterating](http://stackoverflow.com/questions/7340757/c-sharp-list-removing-items-while-looping-iterating) — Rezoan, Jan 28 '16 at 09:47
You could achieve the same thing with a line of LINQ: `list.RemoveAll(x => x.Scanned);` — TVOHM, Jan 28 '16 at 09:47
compuchip ill update the code in the question, and source and see if it helps — Peter, Jan 28 '16 at 09:47
@TVOHM - Just to be picky `list.RemoveAll(x => x.Scanned)` isn't LINQ. It predates LINQ. It's simply an anonymous method cast as a `Predicate` delegate. — Enigmativity, Jan 28 '16 at 09:50
@CompuChip , after the move of item delete the code doesnt hang anymore. You could write an answer and i'll reward you. — Peter, Jan 28 '16 at 09:54
@user3800527 that was _a_ problem, the real problem remains that you are skipping values from the list. E.g. if `i = 0` and you remove the element, then increment to `i = 1` you have missed the element that was originally at the second position (and now at index `i = 0`). The standard solution for removing elements while looping is to iterate the list backwards, I just fixed an additional problem :) — CompuChip, Jan 28 '16 at 09:56
@user3800527 Did your issue get resolved? If so, please mark a reply as the answer. Otherwise, let us know what more is to be fixed, please. — Konrad Viltersten, Jan 28 '16 at 10:12
yes it works now, i rewarded Mohit Shrivastava and also thanks to CompuChip, and thanks Konrad Vilteresten for explaining on how to to do it in Linq, i'm not going to use Linq for this, but still nice example, i'll make a note of it. — Peter, Jan 28 '16 at 10:53

Dmitry Bychenko · Answer 1 · 2016-01-28T10:02:52.440

2

Why not direct List<T>.RemoveAll()?

https://msdn.microsoft.com/en-us/library/wdka673a(v=vs.110).aspx

In your case

  Lijst.RemoveAll(item => some condition);

E.g.

  // Count all the not scanned items each of them exceeds nudMinimum.Value
  lblDebug.Text = Lijst
    .Where(item => !item.scanned && item.price > (int)nudMinimum.Value)
    .Count()
    .ToString();

  // Remove all not scanned items
  Lijst.RemoveAll(item => !item.scanned);

edited Jan 28 '16 at 10:02

answered Jan 28 '16 at 09:52

Dmitry Bychenko

180,369
20
160
215

i still have to do some calculation on it, actually a bit more then in above code so i loop trough them – Peter Jan 28 '16 at 09:56
@user3800527: what is the actual condition, please? – Dmitry Bychenko Jan 28 '16 at 09:57

Mohit S · Accepted Answer · 2016-01-28T09:59:51.447

You might be looking for this

for (int i = Lijst.Count - 1 ; i >= 0 ; i--)
{
    if (Lijst[i].scanned == false)
    {
        if (Lijst[i].price > (int)nudMinimum.Value)
        {
            Totaal++;
            lblDebug.Text = Totaal.ToString();
        }
        Lijst.RemoveAt(i);
    }
}

Question in the comment:

why would the other direction for loop work ?

Because when the loop is run in from Zero to Count There is a situation arise when the index is not available to remove and the count is still left. For example:

if you have 10 items in the List the loop starts at 0 and would remove 0,1,2,3,4 and now the item left are 5 and index is also 5 it would remove that item too. After that when loop value reaches 6 and item left is 4. Then it would create a problem. and it would throw an error. i.e. index out of range

@Byyo yes your right i used the debug label there as a simple way to track it. — Peter, Jan 28 '16 at 11:10

Konrad Viltersten · Answer 3 · 2016-01-28T10:02:07.143

The problems is that when you remove the element number 5, the list gets shorter and the element number 6 is now 5th, number 7 becomes 6th etc. However, if you run the loop backwards, the number is kept as expected.

for(int i = donkeys.Count - 1; i >= 0; i++)
  if(donkeys[i] == some condition here)
    donkeys.RemoveAt(i);

However, it's an like-a-boss approach. There are better ways. You've got the answer but I'd like to suggest a LINQ based approach.

int Totaal = Lijst
  .Where(item => item.scanned)
  .Where(item => item.price > (int)nudMinimum.Value)
  .Count();

Lijst = Lijst.Where(item => !item.scanned).ToList()

Also, as a side note, I wonder if you find the below more readable. Consider the following different naming (both regarding the language and the capitalization).

List<Item> items = ...;
int minimum = (int)nudMinimum.Value;

int total = items
  .Where(item => item.scanned)
  .Where(item => item.price > minimum)
  .Count();

items = items
  .Where(item => !item.scanned)
  .ToList();

hmm i'm not that deep into Linq, i admit that code looks nice — Peter, Jan 28 '16 at 10:00
@user3800527 Yeah, I kind of got that fro the loops. As I said - if you don't dare to go for LINQ (which you should because that's **extremely** useful and everybody uses that), just revert your loop from last element down to zero (not that it's from *.Count -1* and down to *i >= 0*). But if I were you, I would **force** myself to LINQing it. Happy to be of help! :) — Konrad Viltersten, Jan 28 '16 at 10:04

score 1 · Answer 4 · answered Jan 28 '16 at 10:00

1

here you go

// 1. Count items
lblDebug.Text = Lijst.Count(x => x.price > (int)nudMinimum.Value && !x.scanned).ToString();
//2. Remove items
Lijst.RemoveAll(x => !x.scanned);

answered Jan 28 '16 at 10:00

Byyo

2,163
4
21
35

score 1 · Answer 5 · answered Jan 28 '16 at 10:03

First You are removing the element with index i and then using it. You need to first do your process with element having index i and then remove it. Your code will look like below:

for (int i = 0; i < Lijst.Count; i++)
  {
   if (Lijst[i].scanned == false)
    {

     if (Lijst[i].price > (int)nudMinimum.Value)
      {
       Totaal++;
        lblDebug.Text = Totaal.ToString();
      }
Lijst.RemoveAt(i);
    } 
  }

score 0 · Answer 6 · answered Jan 28 '16 at 09:51

0

Normally if you want to remove from a list all items that match a predicate, you'd use List<T>.RemoveAll(), for example:

List<int> test = Enumerable.Range(0, 10).ToList();

test.RemoveAll(value => value%2 == 0); // Remove all even numbers.

Console.WriteLine(string.Join(", ", test));

However, it seems you need to do some additional processing. You have two choices:

Do it in two steps; first use RemoveAll() to remove unwanted items, then loop over the list to process the remaining items separately.
Loop backwards from List.Count-1 to 0 instead.

answered Jan 28 '16 at 09:51

Matthew Watson

104,400
10
158
276

You raise a valid point but I wonder if it meets the OP's requirement on counting the total number of the removees. – Konrad Viltersten Jan 28 '16 at 09:58
@KonradViltersten Hence my comment about doing additional processing (see my choices 1. and 2. in the answer) – Matthew Watson Jan 28 '16 at 10:36

score 0 · Answer 7 · answered Jan 28 '16 at 09:56

your code is some how is not in proper format. first you deleted the list item and then you are trying to catch the price of that deleted item. How can it possible.

so you can write in this way.

    for (int i = 0; i < Lijst.Count; i++)
  {
   if (Lijst[i].scanned == false)
    {
      if (Lijst[i].price > (int)nudMinimum.Value)
      {
       Totaal++;
        lblDebug.Text = Totaal.ToString();
      }
     Lijst.RemoveAt(i);        
    } 
  }

Sajidur Rahman · Answer 8 · 2016-01-28T10:57:47.897

-1

 List<string> list = new List<string>();
         list.Add("sasa");
         list.Add("sames");
         list.Add("samu");
         list.Add("james");
         for (int i = list.Count - 1; i >= 0; i--)
         {

             list.RemoveAt(i);

        }

How to Delete Items from List

edited Jan 28 '16 at 10:57

answered Jan 28 '16 at 09:49

Sajidur Rahman

613
6
9

correct way of looping through a list and remove items

8 Answers8