CODEWITHSUNDEEP
c#if-statement
Diegoctn
Diegoctn

Reputation: 55

"if" that doesn't work perfectly

What wrong with this code? Either combobox is "Wholesale" or "Retail" the form show the second statement (true, true). If I untick the checkbox the else works (true, false). Any idea why? 

private void checkBox3_CheckedChanged(object sender, EventArgs e)
    {
        if (checkBox3.Checked == true || comboBox1.SelectedText == "Standalone")
        {

            this.checkBox1.BackColor = Color.Gray;
            this.checkBox1.Enabled = false;
            this.checkBox2.BackColor = Color.Gray;
            this.checkBox2.Enabled = false;
        }

        if (checkBox3.Checked == true || comboBox1.SelectedText == "Retail")
        {
            this.checkBox1.BackColor = default(Color);
            this.checkBox1.Enabled = true;
            this.checkBox2.BackColor = default(Color);
            this.checkBox2.Enabled = true;
       }
       else
       {
            this.checkBox1.BackColor = default(Color);
            this.checkBox1.Enabled = true;
            this.checkBox2.BackColor = Color.Gray
            this.checkBox2.Enabled = false;
       }

Upvotes: 2

Views: 104

Answers (2)

Dmitrii Bychenko
Dmitrii Bychenko

Reputation: 186833

I suggest eliminating all if here and operate with boolean formulas only. First, let's get rid of pesky (checkBox3.Checked == true || comboBox1.SelectedText == "Standalone"):

 bool isRetail = checkBox3.Checked && comboBox1.SelectedText == "Retail";
 bool isStandalone = checkBox3.Checked && comboBox1.SelectedText == "Standalone";

Then try to guess when CheckBoxs should be Enabled:

 // checkBox2 is enabled if and only if isRetail 
 //TODO: can see, how checkBox2 is unreadable? Change the name into, say "boxIsRetail"
 this.checkBox2.Enabled = isRetail;
 // checkBox2 is enabled if and only if neither isRetail nor isStandalone
 //TODO: can see, how checkBox1 is unreadable? Change the name into, say "boxIsNeither"
 this.checkBox1.Enabled = !(isRetail || isStandalone);

Finally, we should update their Colors. CheckBox's Color depends on Enabled only:

 this.checkBox1.BackColor = this.checkBox1.Enabled ? default(Color) : Color.Gray;
 this.checkBox2.BackColor = this.checkBox2.Enabled ? default(Color) : Color.Gray;

Putting all together we have 

 private void checkBox3_CheckedChanged(object sender, EventArgs e) {
   bool isRetail = checkBox3.Checked && comboBox1.SelectedText == "Retail";
   bool isStandalone = checkBox3.Checked && comboBox1.SelectedText == "Standalone";

   checkBox2.Enabled = isRetail;
   checkBox1.Enabled = !(isRetail || isStandalone);

   //TODO: these lines are good candidate to be extracted into a method "UpdateColor"
   checkBox1.BackColor = checkBox1.Enabled ? default(Color) : Color.Gray;
   checkBox2.BackColor = checkBox2.Enabled ? default(Color) : Color.Gray;
 }

Please, notice, that you can easily add another bool variables like isWholesale, isReserved etc. and implement corresponding logic without roaming among trees of if, else and if else

Upvotes: 1

Fruchtzwerg
Fruchtzwerg

Reputation: 11399

A simple else if and the usage of && should do the trick:

if (checkBox3.Checked == true && comboBox1.SelectedText == "Standalone")
{
    ...
    //If this case is true, the following cases are not going to be checked
}
else if (checkBox3.Checked == true && comboBox1.SelectedText == "Retail")
{
    ...
    //If this case is true, the following cases are not going to be checked
}
else
{
    ...
    //No case before was true
}

Note that the usage of an simple if behind an if is NOT the same as the usage of an else if behind an if.

Also note that your method checkBox3_CheckedChanged only gets called after the state of your third CheckBox was changed (unless you took the method for other events)

Upvotes: 4

Related Questions