Reputation: 43
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
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
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
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