If / else statements in C#

Question

Why does this work?

    private void button1_Click(object sender, EventArgs e)
    {
        if (!checkBox1.Checked)
        {
            MessageBox.Show("The box is not checked!");
        }
        if (checkBox1.Checked == true)
        {
            if (label1.BackColor == Color.Red)
            {
                label1.BackColor = Color.Blue;
            }
            else
            {
                label1.BackColor = Color.Red;
            }
        }
    }

But this doesn't?

   private void button1_Click(object sender, EventArgs e)
    {
        if (!checkBox1.Checked)
        {
            MessageBox.Show("The box is not checked!");
        }
        if (checkBox1.Checked == true)
        {
            if (label1.BackColor == Color.Red)
            {
                label1.BackColor = Color.Blue;
            }
            if (label1.BackColor == Color.Blue)
            {
                label1.BackColor = Color.Red;
            }
        }
    }

I would think that the compliler would read the lines each time I press the button so it should not be any different to have two if statements after each other.

Solution 1

You are changing to blue if it was red and then your are changing it to red if it was blue. Basically first if first if will change it to blue then second if will change it back to red. It works this way because instructions are executed sequentially, so your second if is always checked after your first if. Just use else if so second if won't work if first one fired:

// if red then change to blue
if (label1.BackColor == Color.Red)
{
    label1.BackColor = Color.Blue;
}
// otherwise, if blue then change to red
// this condition will be checked if first "if" was false
else if (label1.BackColor == Color.Blue)
{
    label1.BackColor = Color.Red;
}

Solution 2

Why second snippet do not work.

Because in second snippet both if statements are independent. After color turns blue from first if, the second if gets called and changes its color back to Red. You need an else if

        if (label1.BackColor == Color.Red)
        {
            label1.BackColor = Color.Blue;
        }
        else if (label1.BackColor == Color.Blue)
        {
            label1.BackColor = Color.Red;
        }

Solution 3

This is a logic problem, here is an example of why it doesn't work:

// label is blue at this point
if (label1.BackColor == Color.Blue)
{
    // we change the colour to red
}
// label is red at this point
if (label1.BackColor == Color.Red)
{
    // we change it back to blue
}

The difference between the statements are the first uses else which changes the logic flow so you are only ever dealing with 1 scenario at a time (it's either Red or Blue). However, in the second as they are 2 separate statements you are dealing with both scenarios consecutively. There are a couple of resolutions to this, the most obvious (and probably the best) is to use the else like you are already. However, you could also use a switch:

switch (label1.BackColor)
{
    case Color.Blue:
        label1.BackColor = Color.Red;
        break;
    case Color.Red:
        label1.BackColor = Color.Blue;
        break;
}

You could also keep the code you have in the second snippet but just tell the function you are done by using the return keyword e.g.

if (label1.BackColor == Color.Blue)
{
    // change to red
    return;
}

Or if you want to get really fancy, you could actually refactor the if statement out completely and do it in a one liner - looks pretty ugly though...

label1.BackColor = checkbox1.Checked ? label1.BackColor = Color.Blue ? Color.Red : Color.Blue : label1.BackColor

FYI - if (checkbox1.Checked == true) is a pointless check, just use if (checkbox1.Checked)

Comments

[Catalin]

Yes, just add an else if there

[Smith.Patel]

While showing an alternative way can be useful, you are not actually answering the OP's question of why the first case works, but not the second. If you were to answer the question in addition to the alternative way I'd +1 your answer.

[Smith.Patel]

Depends on if the message that the box is not checked is a requirement.

[James]

@JasonDown the colour should alternate if the checkbox is checked, so in the one-liner if the box isn't checked it just keeps back colour the same. Obviously, if the OP needs to throw a message box up then you wouldn't even need that part as the code would only ever be executed when we are in a "checked" state.