0

As part of a school project, I'm creating a program that simulates a student handing in a test, a student checking his grade, and then a student returning a test. I feel as if I've got the program down, but now VisualBasic is throwing me a InvalidOperationException for my foreach loop because the collection was modified. My program IS working as far as I can tell (I have a debug line that operates as intended).

I'm relatively new at C#, so if I'm off the base anywhere, please let me know. The code is as follows.

First, the error

An unhandled exception of type 'System.InvalidOperationException' occurred in mscorlib.dll

Additional information: Collection was modified; enumeration operation may not execute.

The code (if anyone needs my object class, just say so)

//Stack and Queue calls
Queue submittedTest = new Queue();
Stack outForChecking = new Stack();
private void btnSubmitTest_Click(object sender, EventArgs e)
{
    //generates a random test score
    Random rdm = new Random();
    int testScore = rdm.Next(0, 100);
    string score = testScore.ToString();

    //assigns user input to a variable
    string name = txtName.Text;

    //Generate a new test that passes in 
    Test tests = new Test(name, score);

    //shows the user the name they just enetered
    label3.Text = String.Format("{0}", name);

    //adds submitted test to the queue, then displays that test in a list box
    submittedTest.Enqueue(tests);
    listSubTests.Items.Add(new Test(name, score));

    //Clears input box for next user input
    txtName.Clear();
}
private void btnFindTest_Click(object sender, EventArgs e)
{
    string tempName = txtName.Text;

    foreach (Test tests in submittedTest)
    {
        if(tests.Name == tempName)
        {
            //Remove correct test from submittedTest que
            submittedTest.Dequeue();

            //Add correct test to a new array, outForChecking
            outForChecking.Push(tests);

            //Tester to validate how many items are left in the submittedTest que
            Console.WriteLine("{0}", submittedTest.Count);
        }
    }
}
MarcinJuraszek
  • 124,003
  • 15
  • 196
  • 263
Chris Blackmon
  • 197
  • 4
  • 13
  • 1
    Error message is quite clear - you can't modify a collection while enumerating over it. Also, your logic has one problem - `Dequeue` removes item from the top of the queue, not the one you're currently at while enumerating over it. I think you should not use queues, and use `List` instead. – MarcinJuraszek Sep 07 '15 at 18:12

3 Answers3

1

In C# you can not modify a collection that is being iterated via foreach loop.

Also, as noticed in a comment upwards, the Dequeue method removes the last item of the queue, not the current one (and this probably causes a logical error in your code). Consider changing your submittedTest variable into anything that implements an IList<T> interface (probably List<T> would be nice).

Then you could iterate it like this:

for (int i = submittedTest.Count-1; i>=0; i--)
{
    var tests = submittedTest[i];
    if(tests.Name == tempName)
    {
        //Remove correct test from submittedTest que
        submittedTest.Remove(tests);

        //Add correct test to a new array, outForChecking
        outForChecking.Push(tests);

        //Tester to validate how many items are left in the submittedTest que
        Console.WriteLine("{0}", submittedTest.Count);
    }
}

Notice that this example iterates the collection in a reverse for loop. This is needed to avoid OutOfRangeException when iterating over it.

Community
  • 1
  • 1
bashis
  • 1,200
  • 1
  • 16
  • 35
0

Change this

 foreach (Test tests in submittedTest)
{
    if(tests.Name == tempName)
    {
        //Remove correct test from submittedTest que
        submittedTest.Dequeue();

        //Add correct test to a new array, outForChecking
        outForChecking.Push(tests);

        //Tester to validate how many items are left in the submittedTest que
        Console.WriteLine("{0}", submittedTest.Count);
    }
}

to

for(int i = 0; i < submittedTest.Count - 1; i++)
{

if(submittedTest[i].Name == tempName)
    {
        //Remove correct test from submittedTest que
        submittedTest.Dequeue();

        //Add correct test to a new array, outForChecking
        outForChecking.Push(submittedTest[i]);

        //Tester to validate how many items are left in the submittedTest que
        Console.WriteLine("{0}", submittedTest.Count);
    }
}
  • 1
    [This is going to cause an `OutOfRangeException`](http://stackoverflow.com/questions/1582285/how-to-remove-elements-from-a-generic-list-while-iterating-over-it). Also, your `tests[i]` should probably be `submittedTest[i]`. Also, the `Queue` does not have a `Length` property. – bashis Sep 07 '15 at 18:26
  • bashis is correct, tests does not exist in the current context in the suggestion you gave. – Chris Blackmon Sep 07 '15 at 18:30
0

Neither a Queue nor a Stack seem to be appropriate here. Your collections should both be dictionaries like this:

submittedTest = new Dictionary<string, Test>();
outForChecking = new Dictionary<string, Test>(); 

Then, instead of

submittedTest.Enqueue(tests);

you will write

submittedTest[tests.Name] = tests;

I am assuming that "Name" is the public property on your "object class". Your search click method will look like this:

private void btnFindTest_Click(object sender, EventArgs e)
{
    Test found = null;
    string name = txtName.Text;
    if (submittedTest.TryGetValue(name, out found))
    {
        submittedTest.Remove(name);
        outForChecking [name] = found;
        //Tester to validate how many items are left in the submittedTest que
        Console.WriteLine("{0}", submittedTest.Count);
    }
} 

Your submit routine should perform a similar check. If the student's test is found in the outForChecking dictionary it should be removed and placed back into the submittedTest dictionary.

Lorek
  • 855
  • 5
  • 11