Preventing multiple repeat selection of synchronized Controls ?

Question

The working code sample here synchronizes (single) selection in a TreeView, ListView, and ComboBox via the use of lambda expressions in a dictionary where the Key in the dictionary is a Control, and the Value of each Key is an Action<int>.

Where I am stuck is that I am getting multiple repetitions of execution of the code that sets the selection in the various controls in a way that's unexpected : it's not recursing : there's no StackOverFlow error happening; but, I would like to figure out why the current strategy for preventing multiple selection of the same controls is not working.

Perhaps the real problem here is distinguishing between a selection update triggered by the end-user and a selection update triggered by the code that synchronizes the other controls ?

Note: I've been experimenting with using Delegates, and forms of Delegates like Action<T>, to insert executable code in Dictionaries : I "learn best" by posing programming "challenges" to myself, and implementing them, as well as studying, at the same time, the "golden words" of luminaries like Skeet, McDonald, Liberty, Troelsen, Sells, Richter.

Note: Appended to this question/code, for "deep background," is a statement of how I used to do things in pre C#3.0 days where it seemed like I did need to use explicit measures to prevent recursion when synchronizing selection.

Code : Assume a WinForms standard TreeView, ListView, ComboBox, all with the same identical set of entries (i.e., the TreeView has only root nodes; the ListView, in Details View, has one Column).

private Dictionary<Control, Action<int>> ControlToAction = new Dictionary<Control, Action<int>>();

private void Form1_Load(object sender, EventArgs e)
{
    // add the Controls to be synchronized to the Dictionary
    // with appropriate Action<int> lambda expressions
    ControlToAction.Add(treeView1, (i => { treeView1.SelectedNode = treeView1.Nodes[i]; }));
    ControlToAction.Add(listView1, (i => { listView1.Items[i].Selected = true; }));
    ControlToAction.Add(comboBox1, (i => { comboBox1.SelectedIndex = i; }));

    // optionally install event handlers at run-time like so :

    // treeView1.AfterSelect += (object obj, TreeViewEventArgs evt) 
       // => { synchronizeSelection(evt.Node.Index, treeView1); };

    // listView1.SelectedIndexChanged += (object obj, EventArgs evt) 
       // => { if (listView1.SelectedIndices.Count > 0)
               // { synchronizeSelection(listView1.SelectedIndices[0], listView1);} };

    // comboBox1.SelectedValueChanged += (object obj, EventArgs evt)
       // => { synchronizeSelection(comboBox1.SelectedIndex, comboBox1); };
}

private void synchronizeSelection(int i, Control currentControl)
{
    foreach(Control theControl in ControlToAction.Keys)
    {
        // skip the 'current control'
        if (theControl == currentControl) continue;

        // for debugging only
        Console.WriteLine(theControl.Name + " synchronized");

        // execute the Action<int> associated with the Control
        ControlToAction[theControl](i);
    }
}

private void treeView1_AfterSelect(object sender, TreeViewEventArgs e)
{
    synchronizeSelection(e.Node.Index, treeView1);
}

private void listView1_SelectedIndexChanged(object sender, EventArgs e)
{
    // weed out ListView SelectedIndexChanged firing
    // with SelectedIndices having a Count of #0
    if (listView1.SelectedIndices.Count > 0)
    {
        synchronizeSelection(listView1.SelectedIndices[0], listView1);
    }
}

private void comboBox1_SelectedValueChanged(object sender, EventArgs e)
{
    if (comboBox1.SelectedIndex > -1)
    {
        synchronizeSelection(comboBox1.SelectedIndex, comboBox1);
    }
}   

background : pre C# 3.0

Seems like, back in pre C# 3.0 days, I was always using a boolean flag to prevent recursion when multiple controls were updated. For example, I'd typically have code like this for synchronizing a TreeView and ListView : assuming each Item in the ListView was synchronized with a root-level node of the TreeView via a common index :

    // assume ListView is in 'Details View,' has a single column, 
        //  MultiSelect = false 
        //  FullRowSelect = true
        //  HideSelection = false;

    // assume TreeView 
        //  HideSelection = false 
        //  FullRowSelect = true

    // form scoped variable
    private bool dontRecurse = false;

    private void treeView1_AfterSelect(object sender, TreeViewEventArgs e)
    {
        if(dontRecurse) return;

        dontRecurse = true;
            listView1.Items[e.Node.Index].Selected = true;
        dontRecurse = false;
    }

    private void listView1_SelectedIndexChanged(object sender, EventArgs e)
    {
        if(dontRecurse) return

        // weed out ListView SelectedIndexChanged firing
        // with SelectedIndices having a Count of #0
        if (listView1.SelectedIndices.Count > 0)
        {
            dontRecurse = true;
                treeView1.SelectedNode = treeView1.Nodes[listView1.SelectedIndices[0]];
            dontRecurse = false;
        }
    }

Then it seems, somewhere around FrameWork 3~3.5, I could get rid of the code to suppress recursion, and there was was no recursion (at least not when synchronizing a TreeView and a ListView). By that time it had become a "habit" to use a boolean flag to prevent recursion, and that may have had to do with using a certain third party control.

Answer 1

I believe your approach is totally fine. If you want something a little more advanced, see Rein in runaway events with the "Latch", which allows for

void TabControl_TabSelected(object sender, TabEventArgs args)
{
    _latch.RunLatchedOperation(
        delegate
        {
            ContentTab tab = (ContentTab)TabControl.SelectedTab;
            activatePresenter(tab.Presenter, tab);                       
        });
}

Answer 2

Note: I always assumed an SO user should never answer their own question. But, after reading-up on SO-Meta on this issue, I find it's actually encouraged. Personally, I would never vote on my own answer as "accepted."

This "new solution" uses a strategy based on distinguishing between a control being updated as a result of end-user action, and a control being updated by synchronizing code: this issue was mentioned, as a kind of "rhetorical question," in the original question.

I consider this an improvement: it works; it prevents multiple update calls; but, I also "suspect" it's still "not optimal": appended to this code example is a list of "suspicions."

// VS Studio 2010 RC 1, tested under Framework 4.0, 3.5

using System;
using System.Collections.Generic;
using System.Windows.Forms;

namespace SynchronizationTest_3
{
    public partial class Form1 : Form
    {
        private readonly Dictionary<Control, Action<int>> ControlToAction = new Dictionary<Control, Action<int>>();

        // new code : keep a reference to the control the end-user clicked
        private Control ClickedControl;

        public Form1()
        {
            InitializeComponent();
        }

        private void Form1_Load(object sender, EventArgs e)
        {
            ControlToAction.Add(treeView1, (i => { treeView1.SelectedNode = treeView1.Nodes[i]; }));
            ControlToAction.Add(listView1, (i => { listView1.Items[i].Selected = true; }));
            ControlToAction.Add(comboBox1, (i => { comboBox1.SelectedIndex = i; }));

            // new code : screen out redundant calls generated by other controls 
            // being updated

            treeView1.AfterSelect += (obj, evt)
            =>
            {
             if (treeView1 == ClickedControl) SynchronizeSelection(evt.Node.Index);
            };

            listView1.SelectedIndexChanged += (obj, evt)
            =>
            {
              if (listView1.SelectedIndices.Count > 0 && listView1 == ClickedControl)
              {
                  SynchronizeSelection(listView1.SelectedIndices[0]);
              }
            };

            comboBox1.SelectedValueChanged += (obj, evt)
            =>
            {
              if (comboBox1 == ClickedControl) SynchronizeSelection(comboBox1.SelectedIndex);
            };

            // new code here : all three controls share a common MouseDownHandler
            treeView1.MouseDown += SynchronizationMouseDown;

            listView1.MouseDown += SynchronizationMouseDown;

            comboBox1.MouseDown += SynchronizationMouseDown;

            // trigger the first synchronization
            ClickedControl = treeView1;
            SynchronizeSelection(0);
        }

        // get a reference to the control the end-user moused down on
        private void SynchronizationMouseDown(object sender, MouseEventArgs e)
        {
            ClickedControl = sender as Control;
        }

        // revised code using state of ClickedControl as a filter

        private void SynchronizeSelection(int i)
        {
            // we're done if the reference to the clicked control is null
            if (ClickedControl == null) return;

            foreach (Control theControl in ControlToAction.Keys)
            {
                if (theControl == ClickedControl) continue;

                // for debugging only
                Console.WriteLine(theControl.Name + " synchronized");

                ControlToAction[theControl](i);
            }

            // set the clicked control to null
            ClickedControl = null;
        }
    }
}

Why I "suspect" this is not optimal:

1. the idiosyncratic behavior of WinForms controls has to be taken into account: for example, the ListView Control fires its Selected### Events before it fires a Click Event: ComboBox and TreeView fire their Click Events before their SelectedValueChanged and AfterSelect Events respectively: so had to experiment to find that using 'MouseDown would work the same across all three controls.

2. a "gut level" feeling that I've gone "too far" out on "some kind of limb" here: a sense a much simpler solution might be possible.