1

I have multiple TabItems in my TabControl; tabItem1, tabItem2, tabItem3...these are

CloseableTabItem.

If I add a node in tabItem1 and press a button to make a subGraph model for this node, the

same node should appear in tabItem2 with a button; so that

tabItem2-Header = nodeName and nodeName = tabItem1-Header.

if i press the button from the node in tabitem2, tabitem1 should be focused. if i close

tabItem1 and press the same Button tabItem1 should be loaded again(this happen in

SubGraphButton_Click).

Do you see a problem with this code?

  private void ChildNode_Click(object sender, RoutedEventArgs args)
  {
        System.Windows.Controls.Button button = (System.Windows.Controls.Button)sender;
        Node node = Part.FindAncestor<Node>(button);
        MyNodeData nodeData = node.Data as MyNodeData;
        foreach (TabItem item in tabControl.Items)
        {
            if (nodeData.Text == item.Header.ToString())
            {
                item.Focus();
            }
            else if (nodeData.Text != item.Header.ToString())
            {
                SubGraphButton_Click(sender, args);
            }
        }
 }
 private void SubGraphButton_Click(object sender, RoutedEventArgs args)
 {
        string activeDirectory = @"X:\SubGraph\";
        string[] files = Directory.GetFiles(activeDirectory);
        foreach (string fileName in files)
        {
            FileStream file = new FileStream(fileName, FileMode.Open, FileAccess.Read, FileShare.Read);
            System.Windows.Controls.Button button = (System.Windows.Controls.Button)sender;
            Node node = Part.FindAncestor<Node>(button);
            MyNodeData nodeData = node.Data as MyNodeData;
            if (node != null)
            {
                if (nodeData.Text + ".epk" == fileName.Substring(12, fileName.Length - 12) && !tabControl.Items.Contains(tabItem1))
                {
                    tabControl.Items.Add(tabItem1);
                    tabItem1.Focus();
                    var model = new MyGraphLinksModel();
                    model.Modifiable = true;
                    model.HasUndoManager = true;
                    activateDiagram(myDiagram1);
                    activeDiagram.Model = model;
                    model.Name = fileName.Substring(12, fileName.Length - 12);
                    model.Name = model.Name.Substring(0, model.Name.Length - 4);
                    tabItem1.Header = model.Name;
                    activeDiagram.PartManager.UpdatesRouteDataPoints = false;
                    StreamReader reader = new StreamReader(file);
                    string contents = reader.ReadToEnd();
                    XElement root = XElement.Parse(contents);
                    activeDiagram.LayoutCompleted += LoadLinkRoutes;
                    model.Load<MyNodeData, MyLinkData>(root, "MyNodeData",   "MyLinkData");
                }
           }
  }
1
  • 1
    You should not use an event like a method. Refactor the code and create a new method with all the code inside your SubGraphButton_Click. Then you can use this method from wherever you like. Jul 19, 2012 at 14:54

6 Answers 6

11

When you modify a collection when it is in the middle of being modified it is rather likely to cause errors. The types of errors, and their likeliness, tend to vary based on what the underlying collection actually is. Modifying a List when iterating it is very likely to give you lots of off by one errors (or off by more than one if you modify it a lot) and potentially out of bounds errors. Modifying a LinkedList could result in null pointer exceptions, infinite loops, accessing non-existent items, etc. but is quite a bit less likely.

Because the chances of problems, in the general case, are rather high, the impact of those problems is also rather high, and the difficulty in diagnosing what actually went wrong (and where) C# chooses to just throw an exception whenever you try to iterate a collection that was modified during the iteration. This way you don't end up with weird, unexpected problems that don't manifest themselves until some time much further down the road then where their root cause is.

There are several different strategies that can be used to avoid this issue:

  1. Iterate over a different collection than the one you really want to modify. In some cases this can simply be done by adding a ToList call on a sequence so that it is moved to a new collection; when doing this the collection being iterated is separate from the one being modified and so there is no error.

  2. You can avoid modifying the actual collection inside of the foreach loop. Common examples of this are creating a List or other collection of "changes to make" whether it's itemsToAdd, itemsToRemove etc. Then you can add/remove/whatever for all of those items after the loop. (This is effective if you are only modifying a small percentage of the size of the collection.)

  3. Certain types of collections can be "iterated" without actually using a traditional iterator (meaning a foreach loop). As an example, you can iterate through a List using a regular for loop instead, and simply modify (increment/decrement) the loop variable whenever you add or remove items. This, when done correctly, tends to be an efficient option, but it's quite easy to make a mistake and get something wrong so while the other options are (marginally) less efficient, they are very good options for non-performance intensive code due to their simplicity.

0
2

you can't modify collection you're iterating on.

you can replace the "foreach" loop with simple "for" loop but notice the index you're running on when adding/removing items from the collection.

like this:

        for (int i = 0; i < tabControl.Items.Count; i++)
        {
            TabItem item = tabControl.Items[i];
            ... // your logic here
        }

another option which might be convenient, is instead of adding the items into the tab control.Items collection is getting it as return value, save them in a list and after you've done iterating all the items, insert all the tabs you've created into the items collection so you're not modifying the collection while you're running on it.

1
  • i tried your code it open the file that i need but in more than one tabItem
    – User1979
    Jul 19, 2012 at 15:18
1

You are not permitted to modify a collection (tabControl.Items in this case) while you are enumerating it (which you are doing in your foreach loop) as it will make the enumerator invalid.

The specific line of code which is causing the error is likely to be

// In SubGraphButton_Click
// This line of code is called inside an enumeration of tabControl.Items
// This is not permitted!
tabControl.Items.Add(tabItem1);

Conceptually, your code looks like this:

private void ChildNode_Click(object sender, RoutedEventArgs args)
{
    System.Windows.Controls.Button button = (System.Windows.Controls.Button)sender;
    Node node = Part.FindAncestor<Node>(button);
    MyNodeData nodeData = node.Data as MyNodeData;
    foreach (TabItem item in tabControl.Items)
    {
        if (nodeData.Text == item.Header.ToString())
        {
            item.Focus();
        }
        else if (nodeData.Text != item.Header.ToString())
        {
            // This line will throw an exception 
            DoSomethingThatModifiesTabControlItemsCollection()
        }
    }
 }
1

Inside the foreach loop you call SubGraphButton_Click which in turn adds a new node tabControl.Items.Add(tabItem1); This is not allowed. You can use a for loop instead.

1

Yes, this line

tabControl.Items.Add(tabItem1); 

changes the collection on which you enumerate in the NodeClick
and this is no-no in the enumeration world

Try to loop with a standard for, but in reverse order......

    for( int x = tabControl.Items.Count - 1; x>= 0; x--)
    {  
        TabItem item = tabControl.Items[x];
        if (nodeData.Text == item.Header.ToString())  
        {  
            item.Focus();  
        }  
        else if (nodeData.Text != item.Header.ToString())  
        {  
            SubGraphButton_Click(sender, args);  
        }  
    }  

Looping in reverse order avoid to examine the new items added inside the SubGraphButton.
I don't know if this is a desidered effect or not.

1
  • i tried your code it open the file that i need but in more than one tabItem
    – User1979
    Jul 19, 2012 at 15:17
0

When you ForEach over the TabItems in tabControl, you can't do anything inside the ForEach that will cause the tabControl's items collection to change.

This is a limitation of the framework. It is because you are currently iterating over the TabItems.

So inside of your ChildNode_Click function,

Inside of your ForEach

foreach (TabItem item in tabControl.Items)

you make a call to

SubGraphButton_Click(sender, args);

Inside of that function, you make a call to

tabControl.Items.Add(tabItem1);

You can't manipulate the Items collection while inside the ForEach.

3
  • What exactly modifies the collection? Jul 19, 2012 at 14:52
  • 1
    tabControl.Items.Add(tabItem1); in the function SubGraphButton_Click, which is called in your ChildNode_Click's ForEach block.
    – Scen
    Jul 19, 2012 at 14:55
  • I've just asked because you haven't mentioned it in the answer. Jul 19, 2012 at 14:58

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.