Winforms tools for selection of items using ComboBox - binding datasource

Question

This is mi UserControl containing ComboBox and few buttons for selecting and manipulating collection of items which implement simple interface IHasHeader (just header with name and ID).

It works now but I feel that I use some not ideal hacks, especially in the area of binding of ComboBox's datasource. I'll be glad for any review notes.

public partial class TemplateSelector : UserControl
{
 /// <summary>
 /// Type of access right
 /// </summary>
 public enum TemplateType
 {
 Private,
 Public
 }
 private BindingSource bindSrc = new BindingSource();
 private IHasHeader SelectedItem;
 private bool doHandleSelection = true;
 // delegates
 public delegate void ItemSelectedHandler(IHasHeader item);
 public delegate void NewTemplateSelectionHandler(string name, bool isPublic);
 public delegate void NameSelectionHandler(string name);
 // events
 public event ItemSelectedHandler TemplateSelectionChanged = delegate { };
 public event NameSelectionHandler TemplateDeleting = delegate { };
 public event NewTemplateSelectionHandler CreatingNewTemplate = delegate { };
 public event Action RefreshingTemplatesList = delegate { };
 public event Action ClearingTemplate = delegate { };
 public TemplateType SelectedTemplateType { get; set; }
 public IEnumerable<IHasHeader> TemplatesDatasource
 {
 set
 {
 doHandleSelection = false;
 bindSrc.DataSource = value;
 cboxTemplates.DataSource = bindSrc;
 cboxTemplates.SelectedIndex = -1;
 doHandleSelection = true;
 }
 }
 public TemplateSelector()
 {
 InitializeComponent();
 cboxTemplates.SelectedIndexChanged += cboxTemplates_SelectedIndexChanged;
 }
 /// <summary>
 /// Select given item
 /// </summary>
 /// <param name="item"></param>
 public void SelectItem(IHasHeader item)
 {
 for (int i = 0; i < cboxTemplates.Items.Count; ++i)
 {
 if ((cboxTemplates.Items[i] as IHasHeader).Header.ID == item.Header.ID)
 {
 SelectedItem = item;
 cboxTemplates.SelectedIndex = i;
 return;
 }
 }
 }
 /// <summary>
 /// Clear selection of combobox
 /// </summary>
 public void ClearSelection()
 {
 cboxTemplates.SelectedIndex = -1;
 SelectedItem = null;
 }
 /// <summary>
 /// Add item to combobox datasource
 /// </summary>
 /// <param name="item"></param>
 public void AddItem(IHasHeader item)
 {
 var newSource = (bindSrc.DataSource as IEnumerable<IHasHeader>).ToList();
 newSource.Add(item);
 TemplatesDatasource = newSource;
 }
 private void btnDelete_Click(object sender, EventArgs e)
 {
 if (cboxTemplates.SelectedItem is IHasHeader)
 {
 IHasHeader selected = cboxTemplates.SelectedItem as IHasHeader;
 TemplateDeleting(selected.Header.Name);
 bindSrc.ResetBindings(false);
 }
 ClearSelection();
 ClearingTemplate();
 }
 private void btnNew_Click(object sender, EventArgs e)
 {
 CreatingNewTemplate(TextBoxNewTemplateNew.Text, RadioButtonMeasureQuantitySaveSettingsPublic.Checked);
 bindSrc.ResetBindings(false);
 }
 private void cboxTemplates_SelectedIndexChanged(object sender, EventArgs e)
 {
 if (cboxTemplates.SelectedItem is IHasHeader && doHandleSelection)
 {
 IHasHeader selected = cboxTemplates.SelectedItem as IHasHeader;
 TemplateSelectionChanged(selected);
 }
 }
 private void btnRefresh_Click(object sender, EventArgs e)
 {
 RefreshingTemplatesList();
 bindSrc.ResetBindings(false);
 cboxTemplates.SelectedItem = SelectedItem;
 }
 private void btnClear_Click(object sender, EventArgs e)
 {
 ClearSelection();
 ClearingTemplate();
 }
}

• paparazzo: This could be easier in WPF
• Majak: Yes I can imagine it in WPF. But I want to make existing WinForms version nicer for now as the whole project is written in WinForms.

Answer 1

Style & Conventions

• Enums and delegates shouldn't be nested in a class.

• The default event delegate is the EventHandler. If you want to create a custom event handler then its signature should be

  delegate void ItemSelectedHandler(Object sender, EventArgs e);
  

but if you need custom event args then you should create a new class derived from EventArgs and with the properties that you need:

class SelectEventArgs : EventArgs
{
 ..ctor
 public IHasHeader item { get; }
}

then you can use the generic event-handler:

public event EventHandler<SelectEventArgs> TemplateSelectionChanged = delegate { };

You shouldn't use any other pattern for events like

public event Action RefreshingTemplatesList = delegate { };

If you want to follow the event style and conventions then stick to the EventHandler and event args.

---

Binding

public IEnumerable<IHasHeader> TemplatesDatasource
{
 set
 {
 doHandleSelection = false;
 bindSrc.DataSource = value;
 cboxTemplates.DataSource = bindSrc;
 cboxTemplates.SelectedIndex = -1;
 doHandleSelection = true;
 }
}

here you could write

 bindSrc.DataSource = value.ToList();

instead

bindSrc.DataSource = value;

so that you can later reuse this list for adding more items:

public void AddItem(IHasHeader item)
{
 var headers = bindSrc.DataSource as List<IHasHeader>;
 headers.Add(item);
 bindSrc.ResetBindings(false);
}

Don't abbreviate the names like bindSrc. Use full names and preferably meaninigful ones like headerSource or headerBindingSource. bindSrc doesn't say anything about what the binding source holds.

• Majak: Thanks for your review! Just question, why is this public event Action RefreshingTemplatesList = delegate { }; so bad? I know that it does not follow EventHandler phylosofy, but in this case, when I just need to let client know, that something is happening (without parameters), I feel it like kind of "overkill".
• t3chb0t: @Majak it's a convention and a good practice and like with all conventions you don't have to follow it of course but it's a good thing. Every one knows what to expect from events and how the event arguments are passed. If you invent a new style and constantly change it for every event (once a custom delegate, another time Action yet another time the EventHandler) people will wonder what is this and will need time to learn the new style(s). The choice is yours, I'm just saying ;-)
• Majak: Ok, of course I totally agree that it's good practice to follow conventions :-) Just wanted to know if this was "just" about conventions. Thanks!