CODEWITHSUNDEEP
c#
Max Grip
Max Grip

Reputation: 19

Simplify if-else expression

I am trying to manage a 3-result dialog, in if ... else, but I find it hard to use. This is my code:

if (MessageBox.Show("Are you sure you want to exit", "", MessageBoxButtons.YesNoCancel) == DialogResult.Yes) {
   MessageBox.Show("Why?!", "", MessageBoxButtons.RetryCancel);
   Application.Restart();
} else if (MessageBox.Show("Are you sure you want to exit", "", MessageBoxButtons.YesNoCancel) == DialogResult.No) {
    if (MessageBox.Show("OK ^_^! Good Luck!", "", MessageBoxButtons.OK) == DialogResult.OK) 
        Application.Restart();
} else {
    if (MessageBox.Show("Are you sure you want to cancel?", "", MessageBoxButtons.OKCancel) == DialogResult.OK) {
        MessageBox.Show("Ok! Good Luck!");
        Application.Restart();
    } else {
        MessageBox.Show("Error!");
        Application.Restart();
    }
}

Whenever I run it, if I press "No" or "Cancel", it opens a new dialog. How can I avoid that?

Upvotes: 0

Views: 345

Answers (2)

JuanR
JuanR

Reputation: 7803

Your first if statement is checking for a result of Yes:

if (MessageBox.Show("Are you sure you want to exit", "", MessageBoxButtons.YesNoCancel) == DialogResult.Yes)

If the user selects No or Cancel, you move on to the next if statement which produces another MessageBox:

else if (MessageBox.Show("Are you sure you want to exit", "", MessageBoxButtons.YesNoCancel) == DialogResult.No) {

Finally, if the user selects 'Cancel', you get yet another MessageBox:

else {
    if (MessageBox.Show("Are you sure you want to cancel?", "", MessageBoxButtons.OKCancel) == DialogResult.OK) 
     //Other code here...
     }

This is why you see multiple MessageBox instances.

So, you should only show one box, then work with the result. However, the code suggests that additional information must be gathered on certain instances:

var result = MessageBox.Show("Are you sure you want to exit", "", MessageBoxButtons.YesNoCancel);
switch(result)
{
    case DialogResult.Yes:
        //Another box pops up to ask the user why
        MessageBox.Show("Why?!", "", MessageBoxButtons.RetryCancel);
        Application.Restart();
        break;
    case DialogResult.No:
        //Informational box
        MessageBox.Show("OK ^_^! Good Luck!", "", MessageBoxButtons.OK);
        Application.Restart();
    default:
        //Assume Cancel to be the default behavior, 
        //Pick any value to be the default. It's up to you.
        //Make sure they really, REALLY want to cancel
        if (MessageBox.Show("Are you sure you want to cancel?", "", MessageBoxButtons.OKCancel) == DialogResult.OK) 
        {
            MessageBox.Show("Ok! Good Luck!");
            Application.Restart();
        } 
        else 
        {
            MessageBox.Show("Error!");
            Application.Restart();
        }
}

I don't think your code should be calling Application.Restart at the false branch of the Cancel alternative, but the logic is up to you.

As others have pointed out, please try to properly indent/format your code. It will make it easier for you and others to understand.

Upvotes: 3

roozbeh S
roozbeh S

Reputation: 1124

You should first get the DialogResult and then use it in the if-else statement:

DialogResult result = MessageBox.Show("Are you sure you want to exit", "", MessageBoxButtons.YesNoCancel);

if(result == DialogResult.Yes)
{
    //Code if Ok
}
else if(result == DialogResult.No)
{
    //Code if No
}
else
{
    //Code if Cancel
}

This way the MessageBox will open only once

Upvotes: 7

Related Questions