Efficient way to avoid duplicated code of buttons clicks

Question

Say I have 2 buttons witch supposed to perform the same operation but on different objects.

Currently I'm passing all the needed references to the method like this:

    private void sub1_add_to_db_btn_Click(object sender, EventArgs e)
    {
        Add_Substance_To_Database(
            substanse1, sub1_add_to_db_btn, sub2_add_to_db_btn, sub1_found_in_db_list,
            sub2_found_in_db_list, false, sub1_listBox, sub2_listBox);
    }

    private void sub2_add_to_db_btn_Click(object sender, EventArgs e)
    {
        Add_Substance_To_Database(
            substanse2, sub2_add_to_db_btn, sub1_add_to_db_btn, sub2_found_in_db_list,
            sub1_found_in_db_list, false, sub2_listBox, sub1_listBox);
    }

I was wondering if there is some other, more efficient way to do that. Thanks.

EDIT:

This is how some of my code looks like and it making me CRAZY!!!

private void sub1_add_to_db_btn_Click(object sender, EventArgs e)
    {
        Add_Substance_To_Database(substanse1, sub1_add_to_db_btn, sub2_add_to_db_btn,
            sub1_found_in_db_list, sub2_found_in_db_list, false, sub1_listBox, sub2_listBox);
    }

    private void sub2_add_to_db_btn_Click(object sender, EventArgs e)
    {
        Add_Substance_To_Database(substanse2, sub2_add_to_db_btn, sub1_add_to_db_btn,
            sub2_found_in_db_list, sub1_found_in_db_list, false, sub2_listBox, sub1_listBox);
    }

    private void sub1_edit_name_btn_Click(object sender, EventArgs e)
    {
        Add_Substance_To_Database(substanse1, sub1_add_to_db_btn, sub2_add_to_db_btn,
            sub1_found_in_db_list, sub2_found_in_db_list, true, sub1_listBox, sub2_listBox);
    }

    private void sub2_edit_name_btn_Click(object sender, EventArgs e)
    {
        Add_Substance_To_Database(substanse2, sub2_add_to_db_btn, sub1_add_to_db_btn,
            sub2_found_in_db_list, sub1_found_in_db_list, true, sub2_listBox, sub1_listBox);
    }

    private void sub1_delete_from_db_btn_Click(object sender, EventArgs e)
    {
        Delete_Substance_From_DB(sub1_listBox,
            sub2_listBox,sub2_list_is_from_file,sub1_delete_from_db_btn,
            sub2_delete_from_db_btn);
    }

    private void sub2_delete_from_db_btn_Click(object sender, EventArgs e)
    {
        Delete_Substance_From_DB(sub2_listBox,
            sub1_listBox,sub1_list_is_from_file,sub2_delete_from_db_btn,
            sub1_delete_from_db_btn);
    }

For example: If I want to delete a substance, I need to delete it from both of the lists and remove it from other lists, change the selection to the next substance etc...

Answer 1

Eventually I followed my own suggestion... I created an object array with all the controllers and added same methods to the controllers event handlers. In the method I simply select the right controller according to the sender.

substance1_controllers = new object[]{
            sub1_main_listbox, sub1_peaks_list,sub1_found_in_db_list,
            sub1_similar_in_db_list, sub1_eigenvector_list,
            sub1_sourse_switch_btn, sub1_folder_btn,sub1_add_to_db_btn,
            sub1_edit_name_btn,sub1_delete_btn, sub1_picture_box,
            chart_peaks.Series[0], chart_compare.Series[0], true, -1};
substance2_controllers = new object[]{
            sub2_main_listbox, sub2_peaks_list, sub2_found_in_db_list,
            sub2_similar_in_db_list,sub2_eigenvector_list,
            sub2_sourse_switch_btn, sub2_folder_btn, sub2_add_to_db_btn,
            sub2_edit_name_btn,sub2_delete_btn, sub2_picture_box, 
            chart_peaks.Series[1],chart_compare.Series[1], true, -1};

It might look like it is harder to maintain that way but personally I found it very comfortable to maintain and use with the help of this table (and it looks great):

        // [0]  - Main Listbox
        // [1]  - Peaks Listbox
        // [2]  - Found in Database Listbox
        // [3]  - Found similar Listbox
        // [4]  - Eigenvectors Listbox
        // [5]  - Switch sourse Button
        // [6]  - Change Folder Button
        // [7]  - Add to Database Button
        // [8]  - Edit Name Button
        // [9]  - Delete Button
        // [10] - Picture Box
        // [11] - Peaks Chart Series
        // [12] - Compare Chart Series
        // [13] - List is from File Boolean
        // [14] - Previous Selected Index

Method Example:

void Delete_Substance_From_DB(object sender, EventArgs e)
    {
        object[] controller;
        object[] other_controller;
        if (((Button)sender).Name == "sub1_delete_btn")
        {
            controller = substance1_controllers;
            other_controller = substance2_controllers;
        }
        else
        {
            controller = substance2_controllers;
            other_controller = substance1_controllers;
        }
    }

Using examle:

if (((ListBox)other_controller[0]).Items.Count != 0)
        {
            if (((ListBox)other_controller[0]).Items.Count == index2)
            {
                ((ListBox)other_controller[0]).SelectedIndex = index2 - 1;
            }
            else
            {
               ((ListBox)other_controller[0]).SelectedIndex = index2;
            }
            Main_Listbox_Index_Changed(((ListBox)other_controller[0]), null);
        }

Answer 2

This is not a case of code duplication at all. Refactoring SUCH case would only make it:

• more complicated
• unreadable

Please see: Any valid reason for code duplication?

EDIT:

If you had any more 'meat' in event handlers that is duplicated, I would suggest maybe something different. But you pulled everything down into Add_Substance_To_Database so you already de-duplicated your code successfully.

Answer 3

event handler may be one for few controls, in sender you have button control, which call event, cast it to Button class and use tham as parametrs of method

var button = sender as Button;

Add_Substance_To_Database( substanse2, button, sub1_add_to_db_btn, sub2_found_in_db_list, sub1_found_in_db_list, false, sub2_listBox, sub1_listBox);

for other parameter you can use property Tag, it's object for developer purpose

Answer 4

Given that you are passing different values to Add_Substance_To_Database what you have now is probably the most maintainable code you can hope for.

You could attach one event handler to both buttons, but you'd have to work out which button was pressed and pass the relevant arguments anyway:

if (sender == button1)
{
    Add_Substance_To_Database(
        substanse1, sub1_add_to_db_btn, sub2_add_to_db_btn, sub1_found_in_db_list,
        sub2_found_in_db_list, false, sub1_listBox, sub2_listBox);
}
else
{
    Add_Substance_To_Database(
        substanse2, sub2_add_to_db_btn, sub1_add_to_db_btn, sub2_found_in_db_list,
        sub1_found_in_db_list, false, sub2_listBox, sub1_listBox);
}

Which gets you exactly nowhere.