Simplify code: if statements for UI control

54 views Asked by At

I'm writing code for an array of ComboBoxes in WPF. The application searches for music scales based on selected chords.

The ComboBoxes on the left are prefixed with "tb" (tone box), the ones on the right "cb" (chord box).

Two columns and three rows of ComboBoxes with a "Search" button on the bottom All of the ComboBoxes have a SelectionChanged event handler which does the following:

  • If a ComboBox selected value is 0 ("- None -"), disable all following ComboBoxes
  • If all "cb" ComboBox is not selected, disable Search button

Here's the code of the event handler:

btnSearch.IsEnabled = false;
if (tbOne.SelectedIndex != 0)
{                  
    cbOne.IsEnabled = true;

    if (cbOne.SelectedIndex != 0)
    {
        btnSearch.IsEnabled = true;
        tbTwo.IsEnabled = true;

        if (tbTwo.SelectedIndex != 0)
        {
            btnSearch.IsEnabled = false;
            cbTwo.IsEnabled = true;

            if (cbTwo.SelectedIndex != 0)
            {
                btnSearch.IsEnabled = true;
                tbThree.IsEnabled = true;

                if (tbThree.SelectedIndex != 0)
                {
                    btnSearch.IsEnabled = false;
                    cbThree.IsEnabled = true;

                    if (cbThree.SelectedIndex != 0)
                    {
                        btnSearch.IsEnabled = true;
                    }
                    else
                    {
                        btnSearch.IsEnabled = false;
                    }
                }
                else
                {
                    btnSearch.IsEnabled = true;
                    cbThree.IsEnabled = false;
                }
            }
            else
            {
                btnSearch.IsEnabled = false;
                tbThree.IsEnabled = false;
                cbThree.IsEnabled = false;
            }
        }
        else
        {
            btnSearch.IsEnabled = true;
            cbTwo.IsEnabled = false;
            tbThree.IsEnabled = false;
            cbThree.IsEnabled = false;
        }
    }
    else
    {
        btnSearch.IsEnabled = false;
        tbTwo.IsEnabled = false;
        cbTwo.IsEnabled = false;
        tbThree.IsEnabled = false;
        cbThree.IsEnabled = false;
    }
}
else
{
    cbOne.IsEnabled = false;
    tbTwo.IsEnabled = false;
    cbTwo.IsEnabled = false;
    tbThree.IsEnabled = false;
    cbThree.IsEnabled = false;
}

As you can see it doesn't look pretty.

Here's the XAML for the UI:

<Grid Width="200">
    <Grid.ColumnDefinitions>
        <ColumnDefinition/>
        <ColumnDefinition/>
    </Grid.ColumnDefinitions>

    <StackPanel Grid.Column="0">
        <ComboBox Name="tbOne" KeyboardNavigation.TabIndex="1" Margin="0,10,2.5,0" SelectedIndex="0" SelectionChanged="selectionChanged" IsEnabled="True">
            <ComboBoxItem Content="- None -"/>
        </ComboBox>
        <ComboBox Name="tbTwo" KeyboardNavigation.TabIndex="3" Margin="0,10,2.5,0" SelectedIndex="0" SelectionChanged="selectionChanged" IsEnabled="False">
            <ComboBoxItem Content="- None -"/>
        </ComboBox>
        <ComboBox Name="tbThree" KeyboardNavigation.TabIndex="5" Margin="0,10,2.5,0" SelectedIndex="0" SelectionChanged="selectionChanged" IsEnabled="False">
            <ComboBoxItem Content="- None -"/>
        </ComboBox>
    </StackPanel>
    <StackPanel Grid.Column="1">
        <ComboBox Name="cbOne" KeyboardNavigation.TabIndex="2" Margin="2.5,10,0,0" SelectedIndex="0" SelectionChanged="selectionChanged" IsEnabled="False">
            <ComboBoxItem Content="- None -"/>
        </ComboBox>
        <ComboBox Name="cbTwo" KeyboardNavigation.TabIndex="4" Margin="2.5,10,0,0" SelectedIndex="0" SelectionChanged="selectionChanged" IsEnabled="False">
            <ComboBoxItem Content="- None -"/>
        </ComboBox>
        <ComboBox Name="cbThree" KeyboardNavigation.TabIndex="6" Margin="2.5,10,0,0" SelectedIndex="0" SelectionChanged="selectionChanged" IsEnabled="False">
            <ComboBoxItem Content="- None -"/>
        </ComboBox>
    </StackPanel>
</Grid>
<Button Content="Search" Margin="0, 10, 0,0" IsEnabled="False" Name="btnSearch"/>

So my question is how can I simplify this tree of if statements? The code works, but something tells me it could be written much better.

Thank you!

1

There are 1 answers

1
Sweeper On

I think you need to use else if statements and approach the problem step by step.

If a ComboBox selected value is 0 ("- None -"), disable all following ComboBoxes

Quite self-explanatory:

searchBtn.IsEnabled = false;
if (tbOne.SelectedIndex == 0) {
    cbOne.IsEnabled = false;
    tbTwo.IsEnabled = false;
    cbTwo.IsEnabled = false;
    tbThree.IsEnabled = false;
    cbThree.IsEnabled = false;
} else if (cbOne.SelectedIndex == 0) {
    tbTwo.IsEnabled = false;
    cbTwo.IsEnabled = false;
    tbThree.IsEnabled = false;
    cbThree.IsEnabled = false;
} else if (tbTwo.SelectedIndex == 0) {
    cbTwo.IsEnabled = false;
    tbThree.IsEnabled = false;
    cbThree.IsEnabled = false;
} else if (cbTwo.SelectedIndex == 0) {
    tbThree.IsEnabled = false;
    cbThree.IsEnabled = false;
} else if (tbThree.SelectedIndex == 0) {
    cbThree.IsEnabled = false;
}

If all "cb" ComboBox is not selected, disable Search button

We can rephrase that to

If a "cb" ComboBox is selected, enable Search button:

if (cbOne.SelectedIndex != 0 || cbTwo.SelectedIndex != 0 || cbThree.SelectedIndex != 0) {
    searchBtn.IsEnabled = true;
}