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

Is there a shorter way to write the following if statements?

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

Problem

I have the following code which uses four if statements to check the value in four different combo boxes.

if (!String.IsNullOrEmpty(tx_cmb_Circuits1.Text.Trim()))
    emp.CircuitEmail_Group_Rels.Add(
        new Circuit_Email_Group_Rel
        {
            Circuits = (Circuits) tx_cmb_Circuits1.SelectedItem,
            Group = emp
        });
if (!String.IsNullOrEmpty(tx_cmb_Circuits2.Text.Trim()))
    emp.CircuitEmail_Group_Rels.Add(
        new Circuit_Email_Group_Rel
        {
            Circuits = (Circuits) tx_cmb_Circuits2.SelectedItem,
            Group = emp
        });
if (!String.IsNullOrEmpty(tx_cmb_Circuits3.Text.Trim()))
    emp.CircuitEmail_Group_Rels.Add(
        new Circuit_Email_Group_Rel
        {
            Circuits = (Circuits) tx_cmb_Circuits3.SelectedItem,
            Group = emp
        });
if (!String.IsNullOrEmpty(tx_cmb_Circuits4.Text.Trim()))
    emp.CircuitEmail_Group_Rels.Add(
        new Circuit_Email_Group_Rel
        {
            Circuits = (Circuits) tx_cmb_Circuits4.SelectedItem,
            Group = emp
        });


Is there are shorter way to do the same. Please review my code.

Solution

Add all items to a list and do a foreach instead.

var buttons = new List
{
    tx_cmb_Circuits1,
    tx_cmb_Circuits2,
    ...
};

foreach ( var button in buttons )
{
    if ( !String.IsNullOrEmpty( button.Text.Trim() ) )
    {
        emp.CircuitEmail_Group_Rels.Add(
            new Circuit_Email_Group_Rel
            {
                Circuits = (Circuits) button.SelectedItem,
                Group = emp
            } );
    }
}


P.S.: Consider giving your variables better names. No real need for abbreviations like tx_cmb nowadays.

Code Snippets

var buttons = new List<Button>
{
    tx_cmb_Circuits1,
    tx_cmb_Circuits2,
    ...
};

foreach ( var button in buttons )
{
    if ( !String.IsNullOrEmpty( button.Text.Trim() ) )
    {
        emp.CircuitEmail_Group_Rels.Add(
            new Circuit_Email_Group_Rel
            {
                Circuits = (Circuits) button.SelectedItem,
                Group = emp
            } );
    }
}

Context

StackExchange Code Review Q#1502, answer score: 13

Revisions (0)

No revisions yet.