Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

overriding Form's OnFormClosing for validation

Tags:

c#

.net

winforms

I'm planning on overriding OnFormClosing (of System.Windows.Forms.Form) to perform validation of the user input to my dialog. If the validation fails, then I will set the FormClosingEventArgs's Cancel property to true:

protected override void OnFormClosing(FormClosingEventArgs e)
{
    if (DialogResult == DialogResult.OK)
    {
        if (!IsDialogInputValid())
        {
            e.Cancel = true;
            return;          // Is not calling the base class OnFormClosing okay here?
        }
    }
    base.OnFormClosing(e);
}

My Question: Should I be calling the base class' OnFormClosing even when I cancel the close (that is, should I remove the early return above)?

My current thinking is that I should not call it, since delegates attached to the FormClosing event wouldn't expect to be called when the dialog itself has decided it is not closing. On the other hand, I'm nervous that perhaps the base class' OnFormClosing does other necessary stuff.

FYI, I'm new to Winforms, so any advice on how I should be performing validation (if this is not the best approach) is appreciated.

Related link: http://msdn.microsoft.com/en-us/library/system.windows.forms.form.onformclosing.aspx

like image 901
Matt Smith Avatar asked Feb 17 '11 22:02

Matt Smith


2 Answers

It looks fine. It is really up to the design of your application and your base form to determine whether or not it is appropriate to call base.OnFormClosing(e) and when to call it(at the start or end) i.e. if you had custom code in the base method.

Looking at the example, is there even a reason to be overriding OnFormClosing? Couldn't/shouldn't you be doing your validation where the DialogResult is set to OK? If the input is not valid, then the dialog shouldn't be set to OK and Close() should not be called. Normally OnFormClosing is used to handle any cleanup and to prompt the user asking them if they really want to cancel their current task (usually used for when they hit the X on the form).

Back to the one of the questions at hand, you are safe in calling return if the inputs are not valid. There's no reason to call base.OnFormClosing() if you're not going to actually close. Whether you want to call base.OnFormClosing() or not is based on what logic you have in your base method.

like image 77
TyCobb Avatar answered Nov 05 '22 02:11

TyCobb


Calling the base.OnFormClosing() method is a very hard requirement, the FormClosing event isn't going to run otherwise. Whether to do so if you already set e.Cancel to true is a judgment call. Should a FormClosing event handler be allowed to override it and set it back to false? If that makes sense to you then don't shortcut. Is it likely that the event writer is going to be mystified by e.Cancel already set to true? Then do shortcut.

I'd choose the latter in this specific case since it is a dialog.

like image 4
Hans Passant Avatar answered Nov 05 '22 01:11

Hans Passant