HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpModerate

Repetitive code driving me crazy!

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
coderepetitivecrazydriving

Problem

Ok, So first I must say that everything I know about coding I have learned on my own in my spare time so bear with me if my code is primitive, but please, I am open to any comments to make me better...

Anyway, as for my question. I have an application I am building in C# for .NET Compact Framework (for an HP iPaq) the program's purpose is to act similarly to a Restaurant POS terminal to "ring up" food orders. it has gotten to the point where some of my code is the same line copied god even knows how many times with only a numerical difference between them. here is an example:

private void button9_Click(object sender, EventArgs e)
    {
        AddItem(buttonNames[0], prices[0]);
    }

    private void button14_Click(object sender, EventArgs e)
    {
        AddItem(buttonNames[1], prices[1]);
    }

    private void button5_Click(object sender, EventArgs e)
    {
        AddItem(buttonNames[2], prices[2]);
    }

    private void button10_Click(object sender, EventArgs e)
    {
        AddItem(buttonNames[3], prices[3]);
    }

    private void button13_Click(object sender, EventArgs e)
    {
        AddItem(buttonNames[4], prices[4]);
    }

    private void button4_Click(object sender, EventArgs e)
    {
        AddItem(buttonNames[5], prices[5]);
    }


or

button9.Text = buttonNames[0];
                button14.Text = buttonNames[1];
                button5.Text = buttonNames[2];
                button10.Text = buttonNames[3];
                button13.Text = buttonNames[4];
                button4.Text = buttonNames[5];
                button11.Text = buttonNames[6];
                button15.Text = buttonNames[7];
                button7.Text = buttonNames[8];
                button12.Text = buttonNames[9];
                button16.Text = buttonNames[10];
                button8.Text = buttonNames[11];


I KNOW there are easier ways to do a lot of the code I have written, I just don't know how to do it.

Solution

Even though the difference is minimal, there IS a difference. So you'll need to make a minimal distinction, depending on the button that was pressed.

What you could do is create a general event for all the buttons and in that event you'll determine the index to be passed to the AddItem() method. The code might look like this:

private void ButtonClick(object sender, EventArgs e)
{
    Button b = (Button)sender;

    switch(b.Name)
    {
        case "button9" : AddItem(buttonNames[0], prices[0]); break;
        case "button14" : AddItem(buttonNames[1], prices[1]); break;
        case "button5" : AddItem(buttonNames[2], prices[2]); break;
        //other buttons...
    }
}


Another solution is to pre-assign the index to the Tag property of your button:

button9.Text = buttonNames[0];
button9.Tag = 0;
//Same for other buttons


This way you can also use a general method like before and use the tag to call the AddItem() method. Like this:

private void ButtonClick(object sender, EventArgs e)
{
    Button b = (Button)sender;
    int index = Convert.ToInt32(b.Tag);
    AddItem(buttonNames[index], prices[index]);
}


Lastly, if you name your buttons in a different way, it will become even more easy to do this. Place the index to be used in the name of the button and get it from the name in the general event. Example:

//Assignment of the buttons:
button0.Text = buttonNames[0];
button1.Text = buttonNames[1];
button2.Text = buttonNames[2];

//Event:
private void ButtonClick(object sender, EventArgs e)
{
    Button b = (Button)sender;
    int index = Convert.ToInt32(b.Name.Replace("button", ""));
    AddItem(buttonNames[index], prices[index]);
}


Hope this helps!

Code Snippets

private void ButtonClick(object sender, EventArgs e)
{
    Button b = (Button)sender;

    switch(b.Name)
    {
        case "button9" : AddItem(buttonNames[0], prices[0]); break;
        case "button14" : AddItem(buttonNames[1], prices[1]); break;
        case "button5" : AddItem(buttonNames[2], prices[2]); break;
        //other buttons...
    }
}
button9.Text = buttonNames[0];
button9.Tag = 0;
//Same for other buttons
private void ButtonClick(object sender, EventArgs e)
{
    Button b = (Button)sender;
    int index = Convert.ToInt32(b.Tag);
    AddItem(buttonNames[index], prices[index]);
}
//Assignment of the buttons:
button0.Text = buttonNames[0];
button1.Text = buttonNames[1];
button2.Text = buttonNames[2];

//Event:
private void ButtonClick(object sender, EventArgs e)
{
    Button b = (Button)sender;
    int index = Convert.ToInt32(b.Name.Replace("button", ""));
    AddItem(buttonNames[index], prices[index]);
}

Context

StackExchange Code Review Q#27335, answer score: 14

Revisions (0)

No revisions yet.