Question Why does my DialogResult Fire Twice

[SerialCoder]

       if (ds.HasChanges() == true)
            {
 
                DialogResult userChoice = MessageBox.Show("Changes have been made to the current data.\n\n" +
                "If you want to lose the changes and make another selection click 'OK'\n\n" +
                "otherwise to save the changes click 'Cancel' and then click 'Save to Database' ",
                "Save or Lose Changes",
                MessageBoxButtons.OKCancel,
                MessageBoxIcon.Question,
                MessageBoxDefaultButton.Button1);
 
                if (userChoice == DialogResult.OK)
                {
                    ds.RejectChanges();
                }
                if(userChoice == DialogResult.Cancel)
                {
                    cmbLettings.SelectedIndex = mLettingIndex;
                    return;
                }
 
            }

 

[Skydiver]

DialogResult has no events. It cannot fire even once, much less twice.

Are sure you are asking about DialogResult? Is the code the your presented above living in some kind of event handler? If so, then it is likely that event maybe firing multiple times, but without knowing what event(s) you are handling it is kind of hard to help you.

 

[cjard]

Aside from your problem, you seem to be labouring under the misapprehension that your users can read, and want to read. You're also asking them a question that cannnot be answered with the words "OK" or "Cancel" and then having to use a large block of text to redefine what "ok" and "cancel" mean in terms of answering the question

From a UI/UX perspective, all this should be thrown away, or reduced considerably. Generally rather than bothering the user with an incessant stream of nagging about whether they're sure they want to do something, we just strive to provide easy ways for them to undo any destructive actions they might take

Think about your Apple iPhone. When was the last time you saw it throw you a dialog message box with a lengthy explanation of "are you sure you want to back out of this text message you're writing? It's not been saved? To do it press X, to not do it press Y. You pressed X, are you really really sure?" ?

It didn't; it just allows you to back out, and it saves a draft, and when you back in it restores the draft. The user's destructive action is mitigated without bothering them. If they want to clear the box they can tap (x). If they accidentally clear and want to restore, they can shake the phone and choose undo.. These are all exceptions to the work stream that the user can perform they want to; they aren't forced on the user, blocking the user from proceeding

Dialog boxes are incredibly disruptive to a seamless work stream; their use should be minimized or avoided all together.

 

[Skydiver]

And if you must absolutely use a dialog box, the built in MessageBox supports MessageBoxButtons.YesNo or MessageBoxButtons.YesNoCancel. Or you can write a custom dialog box if the built in message box is not sufficient.

But as noted above, avoid dialogue boxes. They are jarring and disruptive. It is usually people making the transition for console based text UI to a GUI who fall back on the console paradigm of user interaction and lose sight of what a graphical user interaction should be like.

 

[jmcilhinney]

You've posted a six-word title and some code. That's not a good post. You've provided no explanation at all and expected us to make all manner of assumptions. You need to provide a FULL and CLEAR explanation of the problem, which includes exactly what you're trying to achieve, how you're trying to achieve it, what happens when you try and exactly how and where that differs from your expectations.

 

[SerialCoder]

DialogResult has no events. It cannot fire even once, much less twice.

Are sure you are asking about DialogResult? Is the code the your presented above living in some kind of event handler? If so, then it is likely that event maybe firing multiple times, but without knowing what event(s) you are handling it is kind of hard to help you.

Sorry, I did not give all the information. I got in too big of a hurry.

The posted code lives inside a combobox event called cmbLettings_SelectedValueChange event.

The mLettingIndex is a member value that is populated by the same combobox but a different event shown below. It is used to reset the value of the combo box.

What I have tried to get this to work was set a flag =0 and increase the count when the DialogResult runs. It would still show the message twice. I also tried removing the delegate for the event and adding it back.

The code reads the message and jumps to a groupBox_Paint event. The code then shows the message, the user clicks a button, the message is read again in code and then the code continues to the if block. From the if block the code jumps to the a groupBox_Paint event again and then returns to read the message jumps back to the a groupBox_Paint event then shows the message the second time.

private void cmbLettings_Enter(object sender, EventArgs e)
        {
            mLettingIndex=cmbLettings.SelectedIndex;
        }

 

[jmcilhinney]

And if you must absolutely use a dialog box, the built in MessageBox supports MessageBoxButtons.YesNo or MessageBoxButtons.YesNoCancel.

Indeed. In this case, the question asked should be:

Would you like to save your changes?

and the options should be:

• Yes: save and continue
• No: continue without saving

and optionally:

• Cancel: don't continue

 

[SerialCoder]

Indeed. In this case, the question asked should be:

and the options should be:

• Yes: save and continue
• No: continue without saving

and optionally:

• Cancel: don't continue

Thank you for all the comments on reducing the text. They are very helpful and I will incorporate them and reduce the dialog.

Can I get more advise or suggestions on preventing the code from firing twice?

 

[Skydiver]

You still have not shown us where that code lives. What event(s) execute that code?

 

[jmcilhinney]

Given that you're only calling Show once in that code, that code is not the issue. I would guess that the issue is that that code is being executed twice. There's no way for us to know how though. That's what the debugger is for. You can place a breakpoint on that code and then see whether it gets hit twice. If it does, you can use the various debugging tools to find out where from and why, e.g. the Call Stack window will show you exactly the course of execution that was taken to get there.

 

[jmcilhinney]

By the way, I don't think anyone else has mentioned that your two separate inner if statements should not be separate. They should be a single if...else structure or maybe even a switch. Whenever you're checking the same variable against multiple values, there's no point doing any further comparisons once you find a match, so you should not have additional if statements that you know will be false.

 

[Skydiver]

For some reason, post #6 on this thread didn't show up until now for me. Anyway, the selected value change event is fired each time the selected value of a combo box is set. I can't recall right now it is only called when a new value is set, or if it called all the time regardless of the value being set is the same or different from the previous value.

As mentioned above in post #10, setting a breakpoint and looking at the callstack will likely tell you a lot about what is happening.

 

[cjard]

And don't fire anything that interacts with a user from a Paint event (don't do anything that takes a long time in a Paint)

reduce the dialog

To be clear, my recommendation was to eliminate it