CODEWITHSUNDEEP
c#.netwinformsevents
Joan Venge
Joan Venge

Reputation: 330872

How to write event handlers for multiple similar controls?

Basically I have certain controls where they do similar things but for different controls using different values. For instance:

public static void DeleteItemsFromList ( object sender, EventArgs e )
{
    ListBox control = null;
    switch ( ( ( Button ) sender ).Name )
    {
        case "EffectsRemove": control = ( ListBox ) ActiveForm [ "EffectsList" ]; break;
        case "LayersRemove": control = ( ListBox ) ActiveForm [ "LayersList" ]; break;
        case "ObjectsRemove": control = ( ListBox ) ActiveForm [ "ObjectsList" ]; break;
    }

    control.Items.Add ( ( ( Button ) sender ).Name )

    string action = null;
    switch ( ( ( CheckButton ) sender ).Name )
    {
        case "EffectsRemove": action = "Effects"; break;
        case "LayersRemove": action = "Layers"; break;
        case "ObjectsRemove": action = "Objects"; break;
    }

    var selectedItem = control.SelectedItem;
    if ( selectedItem == null )
        return;

    Refresh = false;
    UpdateUI ( action );
    Refresh = true;
}

Is this bad practice? Is there a better way to do these kinds of variable event handlers based on similarly behaving controls?

Upvotes: 1

Views: 1613

Answers (4)

Joe Enos
Joe Enos

Reputation: 40383

You may want to consider using a custom control, where you can derive from Button, and add a few properties to each instance of your CustomButton, representing the string and ListBox that you care about. Then you can tie each button to the same event handler, and act on the properties of the CustomButton, without caring about which one it is.

Upvotes: 1

deepee1
deepee1

Reputation: 13196

Here's another approach you could use via some delegates defined while subscribing to the EventHandler. I think it's a bit easier to read this way, but could get out of hand if you added several other conditions (like if RichTextBox, if Combobox, etc.) 

    private void Form1_Load(object sender, EventArgs e)
    {
        button1.Click += new EventHandler(delegate { DoButtonProcessing(this, "EffectsList", null); });
        button2.Click += new EventHandler(delegate { DoButtonProcessing(this, "LayersList", null); });

        button3.Click += new EventHandler(delegate { DoButtonProcessing(this, null, "Effects"); });
        button4.Click += new EventHandler(delegate { DoButtonProcessing(this, null, "Layers"); });
    }

    void DoButtonProcessing(object sender, string list, string action)
    {
        ListBox control = (ListBox)ActiveForm[list]; //can be null here, but your source also allowed that so I assume it's just a snippit.
        control.Items.Add(((Button)sender).Name);
        var selectedItem = control.SelectedItem;
        if (selectedItem == null) return;
        Refresh = false;
        UpdateUI null;
        Refresh = true;

    }

Upvotes: 1

ChaosPandion
ChaosPandion

Reputation: 78252

Personally I find your example leaves too many opportunities for error. Your best bet would be to extract the common functionality to a separate method.

public static void EffectsRemove_Click(object sender, EventArgs e)
{
    DeleteItemsFromList(
        (Button)sender, 
        (ListBox)ActiveForm["EffectsList"], 
        "Effects");
}

public static void LayersRemove_Click(object sender, EventArgs e)
{
    DeleteItemsFromList(
        (Button)sender, 
        (ListBox)ActiveForm["LayersList"], 
        "Layers");
}

public static void ObjectsRemove_Click(object sender, EventArgs e)
{
    DeleteItemsFromList(
        (Button)sender, 
        (ListBox)ActiveForm["ObjectsList"], 
        "Objects");
}

public static void DeleteItemsFromList(
    Button sender, 
    ListBox control, 
    string action)
{
    control.Items.Add(sender.Name);

    var selectedItem = control.SelectedItem;
    if ( selectedItem == null )
        return;

    Refresh = false;
    UpdateUI action;
    Refresh = true;
}

Upvotes: 2

James Michael Hare
James Michael Hare

Reputation: 38397

I can see your desire to want to reuse logic, but it seems to me this kind of makes the code more brittle and harder to maintain, I'd probably favor separate even handlers in this case even if the code is pseudo-similar.

Upvotes: 1

Related Questions