ilian_dimitrov
ilian_dimitrov

Reputation: 43

for loop skip list elements

i have a problem with the code i showed below.

using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;
using System.Text.RegularExpressions;

namespace problem2
{
    class Program
    {
        static void Main(string[] args)
        {

            int[] nums = Console.ReadLine().Split().Select(int.Parse).ToArray();
            List<int> lst = nums.OfType<int>().ToList();
            while (true)
            {
                string des = Console.ReadLine();
                var output = Regex.Replace(des.Split(" ")[0], @"[^0-9a-zA-Z\ ]+", "");
                

                if (output == "Add")
                {
                    string val = Regex.Replace(des, "[^0-9]", "");
                    int value = int.Parse(val);
                    lst.Add(value);
                }
                else if(output == "Remove")
                {
                    string val = Regex.Replace(des, "[^0-9]", "");
                    int value = int.Parse(val);
                    lst.Remove(value);
                }else if(output == "Replace")
                {
                    String result1 = System.Text.RegularExpressions.Regex.Match(des, @"\d+").Value;
                    
                    var lastno = des.Split().Last();
                    List<int> lst2 = lst;
                    

                    for (int i = 0; i < lst2.Count; i++)
                    {
                        if(lst[i] == int.Parse(result1))
                        {
                            lst[i] = int.Parse(lastno);
                            break;
                        }
                    }
                    lst = lst2;
                }
                else if(output == "Collapse")
                {
                    string val = Regex.Replace(des, "[^0-9]", "");
                    int value = int.Parse(val);
              

                    for(int i = 0; i < lst.Count; i+=1)
                    {
                        int element = (int)lst[i];
                        if (element < value)
                        {
                            lst.RemoveAt(i);
                        }
                        else
                        {
                            continue;
                        }
                    }
                    
                    
                }else if (output == "Mort")
                {
                    PrintValues(lst);
                    break;
                }
            }
        }

        public static void PrintValues(IEnumerable myList)
        {
            foreach (Object obj in myList)
                Console.Write("{0} ", obj);
            Console.WriteLine();
        }
    }
}

more precisely with this part of the code:

else if(output == "Collapse")
{
    string val = Regex.Replace(des, "[^0-9]", "");
    int value = int.Parse(val);


    for(int i = 0; i < lst.Count; i+=1)
    {
        int element = (int)lst[i];
        if (element < value)
        {
            lst.RemoveAt(i);
        }
        else
        {
            continue;
        }
    }

I tried with all sorts of options but the loop misses elements. as an example

entrance:
1 2 -1 0 -3 9 8 7 2
Collapse 8
Mort

output:
9 8

but the program gives me this output:

2 0 9 8 2

I tried to see what the problem is through the debugger and there I found that the loop misses elements

Upvotes: 1

Views: 649

Answers (3)

Olivier Jacot-Descombes
Olivier Jacot-Descombes

Reputation: 112762

You are modifying the collection you are iterating through. This moves elements to other indexes and invalidates the i you determine for following elements in the for header. Better loop backwards

for (int i = lst.Count - 1; i >= 0; i--) {
    // Now you can safely remove (or add) elements without affecting 
    // the index of non yet processed elements.
}

You are still modifying the index of elements ahead, but you are moving backwards, now.

In Visual Studio, you can use the forr code snippet to create a reverse for-loop. Type

forr <tab> <tab>.

(and of course there is the for code snippet for a normal for loop. It will increment the index with i++ instead of i += 1.)


Also, if the list is very long and you are removing many elements, it is worth to think about performance. This approach could move around a great number of elements. Copying the elements you want to keep to a new list (initialized with an appropriate initial capacity), will be more performing in this case.

Upvotes: 1

Berkay Yaylacı
Berkay Yaylacı

Reputation: 4513

It should be,

for(int i = 0; i < lst.Count; i+=1)
{
       int element = (int)lst[i];
       if (element < value)
       {
           lst.RemoveAt(i);
           i--;
       }
       else
       {
           continue;
       }
}

As @David784 mentioned at the comments. When you remove element the from the itterating collection. You miss 1 element everytime after you delete the i item.

Upvotes: 1

David784
David784

Reputation: 7474

To elaborate on @Berkay's comment: You're changing a list as you're looping through it. If i is 2 and you remove that element, then the element that used to be at index 3 now becomes 2, and so forth. His method of decrementing i will work. Another way would be to use a linq .Where statement instead:

var newList = lst.Where(e=> e>=value).ToList();

Upvotes: 1

Related Questions