finding an item in a contextMenu 2

[robUK2]

Hello,

VS 2008

I am adding menuItems to a menuContext. However, I only want to allow a maximum of 10 menuItems to be added. This works ok.

However, if a menuItem is about to be added. Which is already contained in the menuContext. Then I want to get the index of this, so that I can remove it. And add in index 0.

The idea is to add phone numbers, and display the most recent one that was dialed. This is why I am adding at index 0. However, if a phone number is already in the menuContext then I need to find it so that I can remove it.

I have tried using the contains, Find, and IndexOf methods, but no use. See comments below.

I have tried using the Dictionary and IList to keep track of the items. However, this doesn't seem be the right solution either.

Many thanks for any suggestions,

//Dictionary<int, string> RedialedItems = new Dictionary<int, string>(); 
        //IList<string> dialedNumbers; 
 
        //Add a new number to the redial history 
        public void AddToRedialHistory(string number) 
        { 
            this.shortcutRedialMenu = new MenuItem(); 
            this.shortcutRedialMenu.Text = number; 
 
             
            //Trying to get an index of the value I am looking for. 
            //int index = this.ctxRedialMenu.MenuItems.IndexOf(shortcutRedialMenu); 
            //index = this.ctxRedialMenu.MenuItems.IndexOfKey("1"); 
            //index = Convert.ToInt16(this.ctxRedialMenu.MenuItems.Contains(shortcutRedialMenu)); 
 
            //Only keep redial history for a maximum of 10 numbers. 
            //if greater then 10, remove the last one index 10. 
            int count = this.ctxRedialMenu.MenuItems.Count; 
            if (count > 9) 
            { 
                this.ctxRedialMenu.MenuItems.RemoveAt(9); 
            } 
 
            //Add to the zero index so that all the number are displayed 
            //in reverse order. Most recent displayed at the top. 
            this.ctxRedialMenu.MenuItems.Add(0, shortcutRedialMenu); 
 
            //Adding them to the dictionary. 
            //this.RedialedItems.Add(this.shortcutRedialMenu.Index, shortcutRedialMenu.Text); 
             
            //Add event handler for clicking on the shortcut menu. 
            this.shortcutRedialMenu.Click += new EventHandler(this.ShortCutRedial_Clicked); 
        }

 

[robUK2]

Hello,

I managed to do it in the following code. I have noticed that if I don't break out of the loop the code will crash. Why is that?

If anyone can find a more elaborate way to do this, i would be happy to learn.

Many thanks,

private void button1_Click(object sender, EventArgs e)
        {
            menu = new MenuItem();
            foreach (MenuItem item in this.contextmenu.MenuItems)
            {
                if (item.Text == textBox1.Text)
                {
                    this.contextmenu.MenuItems.Remove(item);
                    break;
                }
            }
            menu.Text = this.textBox1.Text;
            contextmenu.MenuItems.Add(0, menu);

            this.menu.Click += new System.EventHandler(menuitemRedial_Click);
        }

 

[bledazemi]

Your problem is related to removing items from the MenuItemCollection inside your foreach loop. Microsoft does not advise using foreach statement when changing the contents of a collection.


"The foreach statement is used to iterate through the collection to get the information that you want, but should not be used to change the contents of the collection to avoid unpredictable side effects." - Microsoft C# Reference (foreach) (Notice their choice of words "should" instead of "must")


The problem lies in the way foreach statement is implemented. Foreach is based on functionality provided by the IEnumerator interface that the IEnumerable class provides (in this case MenuItemsCollection). Now it is entirely up to the implementation of this IEnumerator to decide on the fate of the code if any item is removed. I tested your code and it is clear that IEnumerator class of MenuItemsCollection becomes unstable if you remove any items (except the last one). I would imagine most collections would behave the same because the enumerator interface is retreived only once (for optimisations) at the begining of the foreach loop therefore if you change the list the enumerator interface may not be able to detect the change.

In your case, when you remove an item is the Ienumerator of MenuItemsCollection is not notified of the change and will miss one element soon after the one you removed. E.g. if your MenuItemsCollection list contained {1,2,3,4,5,6,7,8,9} and you removed say element 3 then if you would print out the elements as they are accessed you would get {1,2,3,5,6,7,8,9,null}. So removing 3 will cause the MenuItemsCollection enumerator to miss element 4 and jump to 5. In addition to this enumerator will also produce null at the end as it seems to think it has to traverse 9 elements eventhough only 8 remain.

Having said all this the above may not be true for all IEnumerators as pointed out by one of the links below. I would imagine all standard collections provided by Microsoft would have the problem however it should be possible to write Ienumerator that would safely operate even if changes are made within foreach loop. Briefly speaking follow advice above and do not change the collection within foreach. If you must change it then use simple for or while loops and control your counter accordingly.


Perhaps a better solution for this case is not to remove the items, what you need is simply to change the MenuItem.Index property.

        private void button1_Click(object sender, EventArgs e)
        {
            bool alreadyInList = false;
           
            foreach (MenuItem item in this.contextmenu.MenuItems)
            {
               
                if (item.Text == textBox1.Text)
                {
                    alreadyInList = true;
                    item.Index=0; 
                    break; 
                }
            }

            if (!alreadyInList)
            {
                MenuItem menu = new MenuItem();
                menu.Text = textBox1.Text;

                this.contextmenu.MenuItems.Add(0,menu);

                menu.Click += new System.EventHandler(menuitemRedial_Click);
            }
        }

In fact you could potentially change the index property of the menu item to zero soon after they have been used (at end of their click event) once and avoid having a loop at all. E.g.

        void menuitemRedial_Click(object sender, EventArgs e)
        {
            MenuItem menu = (MenuItem)sender;

            //Here your code 

            //Finally change the index to zero to make this first choice
            menu.Index = 0;
        }

The above description was based on C# reference and my understanding of the following links:

http://bytes.com/forum/thread765778.html

http://msdn.microsoft.com/en-us/library/system.collections.ienumerator.aspx

http://www.codeproject.com/KB/cs/sssienumerable.aspx

"It is in our collective behaviour that we are most mysterious" Lewis Thomas

 

[misterstick]

third post? not bad at all.


mr s. <