Avoiding UpdateData

Critique

Doug Harrison took the time to reply to my essay. He raises some interesting points, and one of them shows that I haven't used a number of dialog features which can lead to some problems. I include a slightly edited version of his comments here, so you can read them and make your own choices.

Doug writes:

I haven't read all your essays, but I do have some comments concerning your OnUpdateData essay.

There are some problems with the sequence below:

CMyDialog dlg;
dlg.m_Count = somecounter;
dlg.m_Text = sometext;
dlg.m_Option = someBool;
dlg.DoModal( );
somecounter = dlg.m_Count;
sometext = dlg.m_Text;
someBool = dlg.m_Option;

1. You shouldn't rely on users to initialize the dialog members directly. As with almost every class, it's much better if the ctor has that responsibility.

This is one I disagree with. Only the caller knows what the values are, the default constructor cannot possibly know any details of the calling site. To avoid this, you have to modify the constructor by hand to add the necessary parameters. This, at least the one time I tried it a few versions back, horribly confused the ClassWizard, which proceeded to corrupt the source file.It made me sufficiently nervous that I've not tried it again. The technique of using the constructor, which he shows below (and, which in my opinion as well is a better way to do it), still requires pulling the data out afterward. Microsoft works in an illusory world in which all of these parameters have to be simple scalars, a restriction so brain-dead that I cannot imagine why anyone could ever imagine it is sufficient. I'd rather pass in a pointer to a structure, but then I can't use ClassWizard to deal with the value variables, because it won't parse structure accesses, or anything other than a simple variable. Since I still have to set everything up, and pull it back out, I prefer the symmetry of seeing the assignments. My inclination would be to have a ClassWizard which would allow me to specify a list of parameters which would be passed in as references, and which would update the variables only upon completely successful completion (that is, all validation was passed). In the absence of something useful, I consider the names of the member variables to be initialized to be part of the specification of the interface. Note that only place I ever use the m_ prefix is a member variable which is used to pass parameters to the dialog from the caller; in no other place anywhere in any dialog I write will you ever find an m_ variable that is used for any other purpose. So I disagree with this point, but only because the tools are still primitive. The bottom line is that Microsoft has a long way to go in providing the necessary automation. In the absence of decent automation, I prefer to do it myself. I'm not opposed to automation, just to poor automation.  --jmn

2. If you care about the values of those members upon return from the dialog, you must care what DoModal returns. To see why, consider that if there's a validation error, UpdateData doesn't run to completion when called from OnOK, and the user can subsequently exit the dialog by canceling it. The code above is subject to storing a mix of new and old values in that scenario, despite the user canceling the dialog, depending on which DDV call in DoDataExchange failed, as well as the order of the DDX/DDV calls.

Thus, the code above is better written as:

CMyDialog dlg(somecounter,sometext,someBool);
if (dlg.DoModal() == IDOK)
   {
    // User said OK, and validation succeeded, 
    //so update application state
   }

In this point he is absolutely correct. Since I never use DDV, for what I think are sound reasons, I would never have encountered this case. If you do use DDV, or think you might use DDV in the future, you need to use the more formally correct form he shows here.--jmn

Also, I can't agree with the premise, "Never call UpdateData." The function definitely has its uses, even in modal dialogs. For example, consider a dialog box that (in part) contains some controls that provide an interface to a simple database of (key,item) pairs, such as an STL map. There is a Save button, and pressing it saves the current record.

I must admit that I've never wanted to do this; I tend to favor writing the code explicitly so that I have some idea of what is going on. This is not a "I don't trust automation, and I'm a Real Programmer". It is a "The automation produces something that is far too simplistic to meet my needs". But if you want to do this, then the issue is that the dialog itself, rather than the caller, cares about the state, and this is a valid point. --jmn

3. To write the Save function of your own, you need to fill the dialog data members, and you use UpdateData for that. If you go to the controls, as you suggest, you bypass necessary dialog data validation as you complicate your code with a lot of control querying. Going directly to the controls is valid only in contexts in which you don't care that the dialog data doesn't reflect the current state of the controls.

I agree, if you are using DDV. Since I believe that DDV is deeply flawed in its basic design (it only validates when you click OK, and I don't like the way it works; I prefer to do continuous validation, enabling/disabling OK as the validity of the data changes, and even displaying the status of what is missing, incorrect, or inconsistent in the status line or other location. So I have deliberately chosen to avoid the DDV mechanism in favor of a far more user-friendly (in my estimation) system. Check out my essays on dialog box state maintenance and on a real-time instantly-reactive validation mechanism. If you choose to use DDV, then his point about bypassing the checking is correct. --jmn

4. You need to override OnOK, because you want to give the user the chance to save any changes. So, you write a QuerySave function, which calls Save if the user gives the OK. In general, if you override OnOK, your override must call UpdateData. In this database example, it's also appropriate to override OnCancel and have it go through the QuerySave bit, so OnCancel can also call UpdateData in some scenarios.

I also agree, if you are working in a model in which after clicking OK you ask "Save changes?" and require a confirmation. I've always worked on the model that if the user clicks OK the intent is to save any changes, and if the user didn't want to save the changes, the user would click "Cancel". If you otherwise override OnOK, you can certainly call CDialog::OnOK which implicitly calls UpdateData, since calling CDialog::OnOK eventually invokes the virtual method. --jmn

Aside: When you override OnOK, if your UpdateData call and other validation are successful, the correct way to end the dialog is to call EndDialog, usually as EndDialog(IDOK), instead of calling CDialog::OnOK. This avoids a pointless second call of UpdateData.

Yes, this is correct, if you choose to use this style. Since I choose to not use this style, it has not been a problem for me. One concern I have is that by calling EndDialog, you are stating that you know that the implementation of CDialog::OnOK is just UpdateData followed by EndDialog. This tends to violate the notion of inheritance abstraction, which bothers me. In particular, if you later subclass the dialog (another feature Microsoft doesn't support at all), then the superclass may be the class that does this, and doing an EndDialog explicitly in the subclass will bypass the superclass method and produce incorrect results. But the statement he makes is correct, at least in this version of MFC. --jmn

You might also like...

Comments

Contribute

Why not write for us? Or you could submit an event or a user group in your area. Alternatively just tell us what you think!

Our tools

We've got automatic conversion tools to convert C# to VB.NET, VB.NET to C#. Also you can compress javascript and compress css and generate sql connection strings.

“Weeks of coding can save you hours of planning.”