check values on a dialog

Hello,

I have a MFC dialog, which has a few controls on it
When I press the OK button I would like to check that my edit box isn't empty, if it is I would like to popup a message and to leave the dialog open until the user will enter a value (or press the cancel button) where Should I i implement that (on what function) I have a CString member variable for that CEdit control, but it will have the CEdit content only after CDialog::OnOk (DoDataExchange) will be called

any ideas?

thanks

You usually implement that in your dialog's DoDataExchange(). Look for DoDataExchange, DDX and DDV on MSDN. Prosise also explains it very well.

There are several solutions. One is to override OnOk(). Another solution is to notify the user in real-time as the edit control becomes empty.

Kuphryn

thanks

1. on my DoDataExchange I have this:

DDX_Text(pDX, IDC_EDIT, m_sEditBoxValue);

//------------------------

is there a special function to call or just the CString function isEmpty?

2. I have just moved recently from VC6 to VC.NET and I can't seem to fine the class wizard, how do I override the OnOk function?

thanks again
avi

Quote:

---

Originally posted by avi123
1. on my DoDataExchange I have this:

DDX_Text(pDX, IDC_EDIT, m_sEditBoxValue);

is there a special function to call or just the CString function isEmpty?

---

The DDX_Text macro transfers contents between a contol and a CString variable in both directions. You will have to check pDX->m_bSaveAndValidate and do the test only when it is TRUE. It's up to you whether you do it before you call the macro (and act upon the control) or after you call the macro (and test the string variable, e.g. with IsEmpty()). But as already said, the entire DDX/DDV mechanism is well documented, just read about it to understand the concepts.

Quote:

---

Originally posted by avi123
2. I have just moved recently from VC6 to VC.NET and I can't seem to fine the class wizard, how do I override the OnOk function?

---

It is not a good idea to override OnOK. There might be other situations when you need to perform data valuation (for example, if you make you dialog modeless and add an Apply button, or if your dialog becomes a property page later). In those cases, just to mention a few, you will have to add additional overrides where you perform the same task. DoDataExchange(), in contrast, is guaranteed to be called for any of those situations, so it's the place to do any validation. After all, that's what it is for by design.

Quote:

---

Originally posted by avi123
2. I have just moved recently from VC6 to VC.NET and I can't seem to fine the class wizard...

---

.NET does not have a wizard.
Use Property panel to insert command message handlers, windows message handlers and virtual overrides.

The answer to your question is to use UpdateData(TRUE) in your OnOk to get the value of the edit control. Then if the data is not correct issue an error message, then return from OnOk without calling CDialog::OnOK().

It is that simple. If you look at the MFC source code for CDialog::OnOK(), you will see that it does two things; it calls UpdateData(TRUE) and then it calls EndDialog(IDOK).

Quote:

---

Originally posted by gstercken
It is not a good idea to override OnOK.

---

It definitely is a good idea to override OnOK. There are plenty of places in the documentation that say so and plenty of examples of doing so.

Quote:

---

Originally posted by Sam Hobbs
It definitely is a good idea to override OnOK. There are plenty of places in the documentation that say so and plenty of examples of doing so.

---

But not to do DDX validation, and that's what the question was about. Validation is done in DoDataExchange - straight and simple. In my previous post, I also gave some reasons why this is so.

Quote:

---

Originally posted by Sam Hobbs
The answer to your question is to use UpdateData(TRUE) in your OnOk to get the value of the edit control. Then if the data is not correct issue an error message, then return from OnOk without calling CDialog::OnOK().

---

Yes, that's a complicated (and unsafe) way of doing it. It will also lead to calling DoDataExchange() twice instead of once... I don't see why one would choose such a complicated solution if the easy, straigtforward solution is so simple.

Quote:

---

Originally posted by gstercken
But not to do DDX validation, and that's what the question was about.

---

I don't see where that is asked.

Quote:

---

Originally posted by gstercken
Yes, that's a complicated (and unsafe) way of doing it. It will also lead to calling DoDataExchange() twice instead of once... I don't see why one would choose such a complicated solution if the easy, straigtforward solution is so simple.

---

It is not complicated. How is it unsafe? Also, if EndDialog is used to end the dialog without calling CDialog::OnOk, then DoDataExchange() will not be called twice.

Quote:

---

Originally posted by Sam Hobbs
I don't see where that is asked.

---

Here:

Quote:

---

Originally posted by avi123
When I press the OK button I would like to check that my edit box isn't empty, if it is I would like to popup a message and to leave the dialog open until the user will enter a value (or press the cancel button)

---

That's exactly what we call 'Dialog Data Validation', or DDV, for short. When doing the validation in DoDataExchange(), calling pDX->Fail() will do exactly what the OP requested (displaying a message box, leaving the dialog open etc.). In addition, it does other useful things, like restoring the focus and the selection to the control where the input has to be corrected. When overriding OnOK, those things had to be implemented manually, too.

Quote:

---

Originally posted by Sam Hobbs
It is not complicated.

---

Not overly complicated, of course, but it's not the designed way of doing validation, and it requires more code (an estimated 3x more). That should be enough to rule it out.

Quote:

---

Originally posted by Sam Hobbs
How is it unsafe?

---

As already mentioned, it is unsafe towards further extensions. When changing the dialog to modeless or to a property sheet, the validation code implemented in OnOK would have to be modified, copied or moved. Failing to remember that would lead to subtle errors (like the validation taking place when clicking OK, but not when clicking Apply and then Cancel, or when switching to a different property page.) So I don't understand why you still try to defend the less obvious way of doing things, if there is a well defined place where to do data validation which takes into account all those situations.

Quote:

---

Originally posted by Sam Hobbs
Also, if EndDialog is used to end the dialog without calling CDialog::OnOk, then DoDataExchange() will not be called twice.

---

Yes, but I was obviously talking about the other case: When CDialog::OnOk is called.

Quote:

---

Originally posted by Sam Hobbs
The answer to your question is to use UpdateData(TRUE) in your OnOk to get the value of the edit control. Then if the data is not correct issue an error message, then return from OnOk without calling CDialog::OnOK().

It is that simple. If you look at the MFC source code for CDialog::OnOK(), you will see that it does two things; it calls UpdateData(TRUE) and then it calls EndDialog(IDOK).

---

So, why bother to override OnOK to do exactly what CDialog does?

Quote:

---

Originally posted by JohnCz
So, why bother to override OnOK to do exactly what CDialog does?

---

What he means is something like this:
void CHisDlg::OnOK()
{
UpdateData(TRUE);

if(m_sEditBoxValue.IsEmpty())
{
MessageBox("Edit box empty");
return;
}
else
{
EndDialog(IDOK);
}
}

CDialog::OnOK() does not check the edit box emptiness like what the above did.

However, though this method is correct, I would prefer doing validation of dialog's control within DoDataExchange(). Too bad DDV does not have a macro for checking whether a string is empty, so we still have to insert code explicitly to validate that. But still, it is neater to put the validation code in DoDatatExchange() than OnOK().
:)

No hi did not.
But I wish all would pay better attention to posts, were the answer is already there.

Quote:

---

Originally posted by gstercken
The DDX_Text macro transfers contents between a contol and a CString variable in both directions. You will have to check pDX->m_bSaveAndValidate and do the test only when it is TRUE. It's up to you whether you do it before you call the macro (and act upon the control) or after you call the macro (and test the string variable, e.g. with IsEmpty()). But as already said, the entire DDX/DDV mechanism is well documented, just read about it to understand the concepts.
.
.
.

---

Did you read this?

The matter is that you are doing unnecessary coding duplicating what CDialog class already does and you could do it with less amount of code in DoDataExchange, that can be called from many places as gstercken already mentioned.

Quote:

---

Originally posted by gstercken
That's exactly what we call 'Dialog Data Validation', or DDV, for short.

---

No, "Dialog Data Validation" is one way of validating the data, but it is not the only way. You are entitled to always use DDV for data validation and it is good for you to suggest it as a possibility, but you are insisting that your way is the only correct way.

Quote:

---

Originally posted by gstercken
When doing the validation in DoDataExchange(), calling pDX->Fail() will do exactly what the OP requested (displaying a message box, leaving the dialog open etc.).

---

Is Wombat correct? Does DoDataExchange not validate for an empty string? If so, then DoDataExchange does nt do exactly what is asked for.

Quote:

---

Originally posted by gstercken
In addition, it does other useful things, like restoring the focus and the selection to the control where the input has to be corrected. When overriding OnOK, those things had to be implemented manually, too.

---

When overriding OnOK, a custom error message can be provided, and restoring the focus to the control is just one line of code.

Quote:

---

Originally posted by gstercken
Not overly complicated, of course, but it's not the designed way of doing validation, and it requires more code (an estimated 3x more).

---

Whose estimate?

I really think that not everyone agrees that DDV is the only solution.

Quote:

---

Originally posted by gstercken
As already mentioned, it is unsafe towards further extensions. When changing the dialog to modeless or to a property sheet, the validation code implemented in OnOK would have to be modified, copied or moved. Failing to remember that would lead to subtle errors (like the validation taking place when clicking OK, but not when clicking Apply and then Cancel, or when switching to a different property page.)

---

These are all typical consequences of making modifications to software. There are many things that can be overlooked if a person does not know what to do or if they simply overlook something. It happens. However the modifications relevant here would not be required when converting to modeless; other changes would definitely be required. I don't know what the changes would be required when converting to a property sheet, but I suspect that other changes would be substantial.

Quote:

---

Originally posted by Wombat
However, though this method is correct, I would prefer doing validation of dialog's control within DoDataExchange().

---

You can use DoDataExchange and certainly many do. Certainly many MFC experts do. However I honestly don't understand why it is a mistake to sometimes do validation in OnOk, when it is simple validation, such as checking for no data.

I have browsed through afxdd.h file and these are the DDV routines I got (I am using VC++ 6) :

/////////////////////////////////////////////////////////////////////////////
// Standard Dialog Data Validation routines

// range - value must be >= minVal and <= maxVal
// NOTE: you will require casts for 'minVal' and 'maxVal' to use the
// UINT, DWORD or float types
void AFXAPI DDV_MinMaxByte(CDataExchange* pDX, BYTE value, BYTE minVal, BYTE maxVal);
void AFXAPI DDV_MinMaxShort(CDataExchange* pDX, short value, short minVal, short maxVal);
void AFXAPI DDV_MinMaxInt(CDataExchange* pDX, int value, int minVal, int maxVal);
void AFXAPI DDV_MinMaxLong(CDataExchange* pDX, long value, long minVal, long maxVal);
void AFXAPI DDV_MinMaxUInt(CDataExchange* pDX, UINT value, UINT minVal, UINT maxVal);
void AFXAPI DDV_MinMaxDWord(CDataExchange* pDX, DWORD value, DWORD minVal, DWORD maxVal);
void AFXAPI DDV_MinMaxFloat(CDataExchange* pDX, float const& value, float minVal, float maxVal);
void AFXAPI DDV_MinMaxDouble(CDataExchange* pDX, double const& value, double minVal, double maxVal);

// special control types
void AFXAPI DDV_MinMaxSlider(CDataExchange* pDX, DWORD value, DWORD minVal, DWORD maxVal);
void AFXAPI DDV_MinMaxDateTime(CDataExchange* pDX, CTime& refValue, const CTime* refMinRange, const CTime* refMaxRange);
void AFXAPI DDV_MinMaxDateTime(CDataExchange* pDX, COleDateTime& refValue, const COleDateTime* refMinRange, const COleDateTime* refMaxRange);
void AFXAPI DDV_MinMaxMonth(CDataExchange* pDX, CTime& refValue, const CTime* pMinRange, const CTime* pMaxRange);
void AFXAPI DDV_MinMaxMonth(CDataExchange* pDX, COleDateTime& refValue, const COleDateTime* refMinRange, const COleDateTime* refMaxRange);


// number of characters
void AFXAPI DDV_MaxChars(CDataExchange* pDX, CString const& value, int nChars);

/////////////////////////////////////////////////////////////////////////////

I do not see any DDV routines that check for emptiness of string. So I presume we still need to code explicitly in DoDataExchange(), something like this (yes, gstercken did mentioned, I put down in codes for better visualisation :p ) :

void CMyDlg:: DoDataExchange(CDataExchange* pDX)
{
CDialog:: DoDataExchange(pDX);

//{{AFX_DATA_MAP(CMyDlg)
DDX_Text(pDX, IDC_EDIT1, m_szEdit);
//}}AFX_DATA_MAP

if(pDX->m_bSaveAndValidate)
{
if(m_szEdit.IsEmpty())
{
MessageBox("Failed");
pDX->Fail();
}
}
}

My point is, whether you do validation in OnOK() or DoDataExchange(), both achieve their purpose and its all up to individual preferences. I do prefer using DoDataExchange, but in this case where the validation is a simple one, I think its acceptable to do it in OnOK() also.

Quote:

---

Originally posted by Wombat
I do not see any DDV routines that check for emptiness of string. So I presume we still need to code explicitly in DoDataExchange

---

I am nearly certain that I have had the problem and read that the problem exists that DDV allows no data.

Quote:

---

Originally posted by Wombat
My point is, whether you do validation in OnOK() or DoDataExchange(), both achieve their purpose and its all up to individual preferences. I do prefer using DoDataExchange, but in this case where the validation is a simple one, I think its acceptable to do it in OnOK() also.

---

Yes, it is at least a matter of preferences. I understand and agree that DDV is very useful and effective.

There is one thing I don't understand about the sample code you posted. Would the focus be set to the control that the error message is for? Or is there a way to designate the control that is not shown in the sample code?

Quote:

---

Originally posted by Sam Hobbs
There is one thing I don't understand about the sample code you posted. Would the focus be set to the control that the error message is for? Or is there a way to designate the control that is not shown in the sample code?

---

Haha, its a mistake in my part. U r right, the sample code I written above did not set the focus to the particular control. It should be like this :

void CMyDlg:: DoDataExchange(CDataExchange* pDX)
{
CDialog:: DoDataExchange(pDX);

//{{AFX_DATA_MAP(CMyDlg)
DDX_Text(pDX, IDC_EDIT1, m_szEdit);

// other DDX and DDV routines
..............
..............
..............

//}}AFX_DATA_MAP

if(pDX->m_bSaveAndValidate)
{
if(m_szEdit.IsEmpty())
{
MessageBox("Failed");
pDX->PrepareCtrl(IDC_EDIT1);
pDX->Fail();
}
}
}

Those DDV routines invoked a function called _AfxFailMinMaxWithFormat() when there's a violation during validation. What that function does is simply popup a messagebox telling us the error and then invoked pDX->Fail(). And the Fail() function is responsible for setting the focus and throwing an exception.

So in our case, since we are not using the DDV routines, we have to explicitly call PrepareCtrl() and Fail(), and of course popup our own messagebox.

Please correct me if I am wrong. :)

Quote:

---

There is one thing I don't understand about the sample code you posted. Would the focus be set to the control that the error message is for? Or is there a way to designate the control that is not shown in the sample code?

---

Yes, the focus will be set to the respective control. CDataExchange::Fail function will do this. It's all in MSDN.

Just realize that there is some minor duplication in my previous code, here's the corrected version :

void CMyDlg:: DoDataExchange(CDataExchange* pDX)
{
CDialog:: DoDataExchange(pDX);

//{{AFX_DATA_MAP(CMyDlg)

// other DDX and DDV routines
..............
..............
..............

//}}AFX_DATA_MAP

DDX_Text(pDX, IDC_EDIT1, m_szEdit);
if(pDX->m_bSaveAndValidate)
{
if(m_szEdit.IsEmpty())
{
MessageBox("Failed");
pDX->Fail();
}
}
}

In this way, we don't have to call PrepareCtrl() explicitly, since the DDX routine before it has invoke the function already.

Quote:

---

Originally posted by Sam Hobbs
No, "Dialog Data Validation" is one way of validating the data, but it is not the only way. You are entitled to always use DDV for data validation and it is good for you to suggest it as a possibility, but you are insisting that your way is the only correct way.

---

I didn't say it is the only way. There are always different ways to do something. You can even do data validation in PreTranslateMessage(), for that matter. I just pointed out that DDV was the way MFC provides for data validation, and that DoDataExchange() is a virtual function designed specifically for that purpose - so why not use it? Validation in OnOK is what I often see beginners doing who haven't yet learnt about the DDX/DDV mechanism.

Quote:

---

Originally posted by Sam Hobbs
Is Wombat correct? Does DoDataExchange not validate for an empty string? If so, then DoDataExchange does nt do exactly what is asked for.

---

Yes, he is correct in that the default implementation of DoDataExchange() doesn't check for an empty string (actually, it does nothing at all). But that's not what I said. He's also correct in that among the many predefined DDV_ macros, there is none which checks for an empty string. The solution is to either provide your own DDV_NotEmpty or DDV_MinChars macro, or to simply code it in terms of IsEmpty(). However, the place to do this test is, by design, DoDataExchange(), and it will work regardless of whether the user clicks 'OK', 'Apply' or switches to another property page.

Quote:

---

Originally posted by Sam Hobbs
When overriding OnOK, a custom error message can be provided, and restoring the focus to the control is just one line of code.

---

Even if it was only one line of code (it is not, as you first have to find the offending control(s), set the focus, restore the selection etc.), that single line alone would still be one line too much. Let alone the several lines, message map entry etc. needed to implement OnOK. DoDataExchange(), in contrast, need not be added: It's already there in wizard-generated dialog code.

Quote:

---

Originally posted by Sam Hobbs
Whose estimate?

---

Mine, based on the many, many pieces of code in various projects I have seen which do dialog data validation in either of the ways.

Quote:

---

Originally posted by Sam Hobbs
I really think that not everyone agrees that DDV is the only solution.

---

Again, not the only solution. In coding, there's never one single possible solution to a problem. You can always leave the path provided by the designers of a framework and do things in more complicated and less obvious ways, circumventing ready-made mechanisms which are there exclusively to make your coding work easier. But I don't see a point in doing so unless you have a very good reason.

Quote:

---

Originally posted by Sam Hobbs
These are all typical consequences of making modifications to software. There are many things that can be overlooked if a person does not know what to do or if they simply overlook something. It happens.

---

But one of the points in software design is to try to minimize the probability of those problems arising - not to create them artificially.

Quote:

---

Originally posted by Sam Hobbs
However the modifications relevant here would not be required when converting to modeless;

---

And what about the 'Apply' button? You would have to perform the same test you added to OnOK() in OnApply() too.

Quote:

---

Originally posted by Wombat
I do not see any DDV routines that check for emptiness of string.

---

You're right, there are none. As said above, you have to code them yourself (exactly the way you did in the code you posted). My point was only (and still is) that the most obvious place to do this, according to the design of MFC, is DoDataExchange, not OnOK.

Quote:

---

Originally posted by Wombat
My point is, whether you do validation in OnOK() or DoDataExchange(), both achieve their purpose and its all up to individual preferences. I do prefer using DoDataExchange, but in this case where the validation is a simple one, I think its acceptable to do it in OnOK() also.

---

I thought I pointed out why OnOK() is not an acceptable alternative. Besides the problems mentioned regarding possible future extensions towards modeles dialogs or property pages, it also requires you to locate the offending control, set the focus, set the selection, and prevent the dialog from being closed. CDataExchange::Fail() already does all this for you. And IMO, any solution which requires rewriting code which is already there for you to be used is not an acceptable alternative.

Quote:

---

Originally posted by kuphryn
There are several solutions. One is to override OnOk(). Another solution is to notify the user in real-time as the edit control becomes empty.

Kuphryn

---

Or use DoDataExchange which is designed specifically to handle data validation.

Quote:

---

Originally posted by GCDEF
Or use DoDataExchange which is designed specifically to handle data validation.

---

Mmmh... Have you read the rest of the thread?

Quote:

---

Originally posted by Sam Hobbs
You can use DoDataExchange and certainly many do. Certainly many MFC experts do. However I honestly don't understand why it is a mistake to sometimes do validation in OnOk, when it is simple validation, such as checking for no data.

---

Very simple. There is a function specifically designed for data validation. For the sake of consistency and ease of maintenance, it is best to use the provided functionality, which in this case is also the simplest. The code involved is

if(pDX->m_bSaveAndValidate)
{
if(m_EditString.IsEmpty())
{
AfxMessageBox("Enter some data");
pDX->PrepareEditCtrl(IDC_MYEDIT);
pDX->Fail();
}
}

Very simple. It's always very frustrating when programmers reinvent the wheel when it isn't necessary.

Quote:

---

Originally posted by gstercken
Mmmh... Have you read the rest of the thread?

---

Yes, and you're doing a great job on this one. I frequently see kuphryn presenting complicated solutions to simple problems, so I felt like calling him on this one.

Quote:

---

Originally posted by GCDEF
I frequently see kuphryn presenting complicated solutions to simple problems,

---

That's certainly true. Or 'correct', so to speak.. ;)

Quote:

---

Originally posted by GCDEF
so I felt like calling him on this one.

---

Although I understand you, that doesn't add anything useful to the discussion...

Quote:

---

Originally posted by gstercken
Although I understand you, that doesn't add anything useful to the discussion...

---

It adds another voice to your position, and hopefully will encourage kuphryn to rethink his answers. Maybe that's wishful thinking on my part, but I've been doing this a long time, and one of the single biggest problems we face with maintaining our product is programmers that reinvent the wheel, or introduce complex solutions to simple problems. That invariably adds bugs and increases maintenance time as miscreant code must be tracked down and corrected or removed.

The best advice for beginners is to learn the framework first and understand how it works and what it does, and to use it whenever possible. I get kind of peeved when I see advice that I know will only lead to headaches down the road that someone like me and probably you will have to fix later.

Interesting.

In your example, how often does windows check whether the edit control is empty?

Here is one scenario.

- empty edit control
- user enters "2"
- user erases "2"

// At this point, does your solution calls the dialog window?

- user enters "2"
- user enters "0"
- user enters "0"
- user enters "4"
- user erases "2004"

// At this point, does your solution calls the dialog window?

My point is, what if the developer just wanted to check the edit control only when the user is ready to close the window?

One thing I have to thank you guys is the fact that you guys have proven to me one thing about programmers. Most programmers have huge ego, i.e. my solution is BEST.

As I have mentioned several times, it is up to the person who asked the question to choose what solution to implement. My goal has always been to provide several solutions and allow the person to analyze it individually.

Complicated is a relative term. I honestly think rarely is anything is "complicated." If it can be done by someone, I can certainly try and most often succeed.

Kuphryn

Quote:

---

Originally posted by kuphryn
Here is one scenario.

- empty edit control
- user enters "2"
- user erases "2"

// At this point, does your solution calls the dialog window?

---

No.

Quote:

---

Originally posted by kuphryn
- user enters "2"
- user enters "0"
- user enters "0"
- user enters "4"
- user erases "2004"

// At this point, does your solution calls the dialog window?

---

Again, no.

Quote:

---

Originally posted by kuphryn
My point is, what if the developer just wanted to check the edit control only when the user is ready to close the window?

---

That's exactly what happens in DoDataExchange(). :)

Quote:

---

Originally posted by kuphryn

As I have mentioned several times, it is up to the person who asked the question to choose what solution to implement. My goal has always been to provide several solutions and allow the person to analyze it individually.

Kuphryn

---

But people who come here asking questions such as this obviously don't have much experience. It is up to those of us that do to guide them in the right direction. Sure there are always lots of options, but very seldom are they all ideal. We should strive to provide the simplest most elegant solution, not just throw things out there to those that may lack the experience to choose wisely.

Quote:

---

Originally posted by kuphryn
Interesting.

My point is, what if the developer just wanted to check the edit control only when the user is ready to close the window?

Kuphryn

---

But you suggested "Another solution is to notify the user in real-time as the edit control becomes empty."

Correct.

Note "Another solution."

Kuphryn

Quote:

---

Originally posted by kuphryn
Correct.

Note "Another solution."

Kuphryn

---

But a bad one, so why bring it up?

"Bad" is a relative term depending on the problem and situation.

Anyways, I do not want to talk about this anymore. If you do not like my post, then simply ignore it.

Kuphryn

Quote:

---

Originally posted by kuphryn
"Bad" is a relative term depending on the problem and situation.

Anyways, I do not want to talk about this anymore. If you do not like my post, then simply ignore it.

Kuphryn

---

I'm not going to ignore your posts, just as I hope if I post incorrectly here somebody corrects me.

It is not reasonable to expect that you can post misleading advice and not have it questioned. In many cases, this being one of them, there is a clearly optimal approach.

The purpose of this board is to provide advice and exchange ideas. I see nothing to be gained from providing bad advice to those seeking good advice.

Exchanging ideas is definitely good.

Criticizing ideas is definitely not good and is unproductive.

Kuphryn

Quote:

---

Originally posted by kuphryn
My point is, what if the developer just wanted to check the edit

---

Yes, that is an important and relevant point. That is what the original question is asking how to do; that is, do the validation in OnOk. Perhaps it would be better to not do the validation there, but it is reasonable to do the validation there. Based on requirements, it might be a better solution. I think we should provide an answer to the question as it is stated, then suggest alternatives and/or ask for clarification.

What if there are dependencies among the fields and errors depend on the values of other fields? I think in those situations DDV could get very complicated or DDV would be very inconvenient for the user.

Quote:

---

Originally posted by kuphryn
Exchanging ideas is definitely good.

Criticizing ideas is definitely not good and is unproductive.

Kuphryn

---

What nonsense. What is unproductive is sending a novice off lerarning how to implement a bad idea that won't work anyway.

Quote:

---

Originally posted by Sam Hobbs
Yes, that is an important and relevant point. That is what the original question is asking how to do; that is, do the validation in OnOk.

---

Now this is a good example of how harmful misinformation can be. Let's see what happend: kuphryn, who appearently doesn't have the foggiest about what DDX/DDV is and how it works, seems to believe that DoDataExchange() is called every time a key is pressed inside an edit control. The following remark

Quote:

---

Originally posted by kuphryn
My point is, what if the developer just wanted to check the edit control only when the user is ready to close the window?

---

as a point against using DoDataExchange(), and the examples he gave, asking

Quote:

---

Originally posted by kuphryn
In your example, how often does windows check whether the edit control is empty?

---

clearly show that.

However, contrary to what his statemets seem to suggests, fact is that during the lifetime of a plain modal dialog, DoDataExchange() is called exactly twice: Once after OnInitDialog is called (with pDX->m_bSaveAndValidate = FALSE), and once again at the end from OnOK (this time, pDX->m_bSaveAndValidate = TRUE), so this is the only time when validation is actually performed. Exactly as the OP requested.

It is true that you can invoke validation at any time by calling UpdateData(TRUE) in order to validate your data at a finer granularity, but as long as you don't, validation is carried out exactly once - when OK is clicked.

The problem with the misleading statements made by kuphryn is not only they create useless ramifications of the discussion, but they also manage to make posters who should know better (namely Sam) to adopt that wrong belief. Sam's last post clearly shows that: Probably without reflecting much about it, he quotes that misleading phrase and calls it an "important and relevant point". Yes, what "if the developer just wanted to check the edit control only when the user is ready to close the window"? I gave the answer to that right at the beginning, in the second post to this thread: Implement it in DoDataExchange(). Full stop.

The whole wrong idea about DoDataExchange() being called all the time has been introduced solely by kuphryn's posts which suffered from complete ignorance of the subject. What makes me really worry is how Sam now carries on the discussion based on those wrong, unchecked assumptions.

Given that DoDataExchange() gets called "only twice," then your solution is no better than the one I present in terms of OnOk().

Just because someone at Microsoft designed DoDataExchange(), does not mean everyone has to adhere to it. This is one example.

Kuphryn

Quote:

---

Originally posted by kuphryn
Given that DoDataExchange() gets called "only twice," then your solution is no better than the one I present in terms of OnOk().

---

Read carefully. DoDataExchange() gets called twice, but the validation part (m_bSaveAndValidate=TRUE) gets called only once. Just as in OnOK, but with many additional benefits (and please don't ask again what those additional benefits are - just read this thread again. It's all there).

Quote:

---

Originally posted by kuphryn Just because someone at Microsoft designed DoDataExchange(), does not mean everyone has to adhere to it.

---

I don't see the point in continuing to defend a suboptimal way of doing something if there is an optimal way. I don't know if you are actually developing real software out there, but as a team lead, I would have severe problems with a developer who insists on defending a wrong approach just because he initially came up with it and now can't admit that he was wrong. That kind of attitude introduces real risks for the overall quality of software products (see what GCDEF said on that behalf). The best developers have been proven those who reflect upon a given problem and carefully consider several options before answering, and not those who immediately jump from their seats and shout "I know something, too!".

Quote:

---

Originally posted by gstercken
I didn't say it is the only way.

---

You do by implication. You insist that any other solution is unacceptable.

Quote:

---

Originally posted by gstercken
You can even do data validation in PreTranslateMessage(), for that matter.

---

You are being unrealistic, which is consistent with insisting that any other solution is unacceptable.

Quote:

---

Originally posted by gstercken
I just pointed out that DDV was the way MFC provides for data validation, and that DoDataExchange() is a virtual function designed specifically for that purpose - so why not use it? Validation in OnOK is what I often see beginners doing who haven't yet learnt about the DDX/DDV mechanism.

---

These are critical issues. Specifically, the question is whether DDV is "the" way or "a" way. Another question is whether validation in OnOK is always a bad choice.

Quote:

---

Originally posted by gstercken
However, the place to do this test is, by design, DoDataExchange(), and it will work regardless of whether the user clicks 'OK', 'Apply' or switches to another property page.

---

Again, you insist that DoDataExchange is the only acceptable solution.

Quote:

---

Originally posted by gstercken
Even if it was only one line of code (it is not, as you first have to find the offending control(s), set the focus, restore the selection etc.), that single line alone would still be one line too much. Let alone the several lines, message map entry etc. needed to implement OnOK. DoDataExchange(), in contrast, need not be added: It's already there in wizard-generated dialog code.

---

You are implying that it is not normal to process IDOK (override OnOK).

Quote:

---

Originally posted by gstercken
But one of the points in software design is to try to minimize the probability of those problems arising - not to create them artificially.

---

You are saying that a good programmer will overlook obvious changes when changing a program. I think it is matter of personal preference whether that is important.

Quote:

---

Originally posted by gstercken
You're right, there are none. As said above, you have to code them yourself (exactly the way you did in the code you posted). My point was only (and still is) that the most obvious place to do this, according to the design of MFC, is DoDataExchange, not OnOK.

---

Again, you insist that DoDataExchange is the only acceptable solution.

Quote:

---

Originally posted by gstercken
I thought I pointed out why OnOK() is not an acceptable alternative. Besides the problems mentioned regarding possible future extensions towards modeles dialogs or property pages, it also requires you to locate the offending control, set the focus, set the selection, and prevent the dialog from being closed. CDataExchange::Fail() already does all this for you. And IMO, any solution which requires rewriting code which is already there for you to be used is not an acceptable alternative.

---

Again, you insist that DoDataExchange is the only acceptable solution. The advantages of DoDataExchange that you describe here are a matter of personal preference.

The following are samples of doing validation in OnOk. Are these not examples of acceptable code? Are these things that a good MFC programmer would never do?

Code:

---

void CSpeakNDlg::OnOK() {
CString result;
InputEdit().GetWindowText(result);
if (result != m_targetWord) {
  PlaySound(IDSOUND_INCORRECT);
  AfxMessageBox(IDS_TRY_AGAIN);
  return;
  }
PlaySound(IDSOUND_CORRECT);
AdvanceLesson();
}

---

Code:

---

void CSpinEditDlg::OnOK() {
int values[NUM_EDIT];
UINT nID = 0;
BOOL bOk = TRUE;
for (int i = 0; bOk && i < NUM_EDIT; i++) {
  nID = IDC_EDIT_MIN + i;
  values&#091;i&#093; = GetDlgItemInt(nID, &bOk);
  }
if (!bOk) {
  // report illegal value
  CString str;
  str.LoadString(IDS_ILLEGAL_VALUE);
  MessageBox(str);
  CEdit& badEdit = *(CEdit*)GetDlgItem(nID);
  badEdit.SetSel(0, -1);
  badEdit.SetFocus();
  return;    // don't end dialog
  }
EndDialog(IDOK);
}

---

Code:

---

void CCustListDlg::OnOK() {
int nIndex = m_colors.GetCurSel();
if (nIndex == -1) {
  CString str;
  str.LoadString(IDS_PLEASE_SELECT_COLOR);
  MessageBox(str);
  m_colors.SetFocus();
  return;
  }
DWORD color = m_colors.GetItemData(nIndex);
EndDialog(IDOK);
}

---

Quote:

---

Originally posted by gstercken
The best developers have been proven those who reflect upon a given problem and carefully consider several options before answering, and not those who immediately jump from their seats and shout "I know something, too!".

---

Exactly true. The most dangerous developers I run accross are those that don't know intuitively when they're heading down the wrong path.

Quote:

---

Originally posted by kuphryn
Given that DoDataExchange() gets called "only twice," then your solution is no better than the one I present in terms of OnOk().

Just because someone at Microsoft designed DoDataExchange(), does not mean everyone has to adhere to it. This is one example.

Kuphryn

---

gstercken has already explained, but to clarify even further, the first time it is called, right before the dialog is displayed, it takes data from variables and updates the controls. The second time it takes data from the controls and puts it into the variables.

As you clearly don't understand the mechanism, it may be best to refrain from commenting on it.

Please stay with the topic at hand. The issue here is whether or not to implement the datacheck OnOk() or DoDataExchange(). As I have mentioned before based on what has been said about DoDataExchange().

For this particular problem, the DoDataExchange() solution is not more efficient that OnOk().

Kuphryn

Quote:

---

Originally posted by kuphryn
Please stay with the topic at hand. The issue here is whether or not to implement the datacheck OnOk() or DoDataExchange(). As I have mentioned before based on what has been said about DoDataExchange().

For this particular problem, the DoDataExchange() solution is not more efficient that OnOk().

Kuphryn

---

If you don't know what you're talking about, please don't post. You are wrong, and I'm sorry you don't see that. Play with it a bit and you'll see.

It isn't just a question of effeciency. It's a question of standards and maintainability as well. Convention calls for DoDataExchange (which is easier to use) and other programmers will expect it to be there and will understand how it works. Easy to maintain code is way more important than you seem to give it credit for.

OK, so let me answer once again (hopefully for the last time in this thread) to some of the statements:

Quote:

---

Originally posted by Sam Hobbs
You do by implication. You insist that any other solution is unacceptable
...
Again, you insist that DoDataExchange is the only acceptable solution.
...

---

I said more than once that doing validation in OnOK() is of course possible, but less desirable for various reasons.

You failed so far to show even one single advantage of doing validation in OnOK() as opposed to doing it in DoDataExchange(). In contrast, I mentioned several advantages of using DoDataExchange() (or drawbacks of using OnOK(), depending on the way you prefer to look at it).

Now I always assumed it as reasonable in terms of common sense (my mistake) that given two options A and B, A having only advantages, but no drawbacks as compared to B, that it would be obvious under any circumstances to use A. So while B still remains a possible option, it would simply be silly to choose it. It is clear that, when doing wordplay, a less workable option is still an option and can be employed, but it is simply not useful to do so. So yes, in a context of practical reasoning and common sense (which I thought we were in) the better option is indeed to be considered the only acceptable solution. But I also bent towards the more philosophical point of view (which you chose to adopt) that a less workable option is also a possible solution, in the strict sense of the word. However, any intelligent person wouldn't think twice and consider only the better solution as an acceptable solution. Note that we're not talking about a classical decision problem here where both options have advantages and drawbacks which have to be evaluated against each other. We are talking about a situation where option A has only advantages and no drawbacks as compared to B. In this case, the decision process becomes trivial.

Quote:

---

Originally posted by Sam Hobbs
You are being unrealistic, which is consistent with insisting that any other solution is unacceptable.

---

What's so unrealistic about doing data validation in PreTranslateMessage()? It's possible, and I've seen people doing it. Someone who doesn't know about implementing OnOK() could use PreTranslateMessage() for data validation. It might be absurd, clumsy, error-prone and require a lot of additional code, but it works. So to use your own words, it is "a" way. Likewise, to a person who knows about DDX/DDV and DoDataExchange(), doing the validation in OnOK() seems equally absurd, clumsy, error-prone, and requires additional code. I would even call that approach unrealistic, to reuse that term.

Quote:

---

Originally posted by Sam Hobbs
You are implying that it is not normal to process IDOK (override OnOK).

---

No, I'm not implying that. Of course it is normal to override OnOK(). After all, that's why it is virtual. I only stated that it is the wrong place (or not the best place, if you like that better) to do data validation.

Quote:

---

Originally posted by Sam Hobbs
You are saying that a good programmer will overlook obvious changes when changing a program. I think it is matter of personal preference whether that is important.

---

That's not something I say, but a commonplace in software development. Errors occur, and good design tries to minimize the possibility of them occurring. It's so self-evident that I'm astonished you even question it...

Anyway, I fear that this discussion doesn't lead any further. It is pretty obvious by now that you (and kuphryn too) are merely defending the data-validation-in-OnOK approach for the sake of defending your initial statement, and not because there are any technical arguments in favour of that approach. You both seem to have problems admitting that you were wrong. You could have easily written something like "Sorry, I totally forgot about DoDataExchange... Of course, that's actually the place to do data validation." Likewise, kuphryn could have said "Oh, I never heard about DoDataExchange before. My apologies. Next time, I will try to inform myself better before posting an answer. You always keep learning..." What's so difficult about that? Instead, you both keep insisting - in spite of hard technical facts which stand against it - that OnOK is an equally good choice for implementing data validation. It is not. Read MSDN (especially TN026), read Prosise - and it will become obvious that DDX/DDV is not only the "official" way to do validation, but that there are also good reasons to prefer that way. Most of them have been mentioned in this thread.

I can't and I don't want to prevent you from continuing to do data validation in OnOK in your own projects. Do as you like - after all, it's your team members / code reviewers / testers / customers who will complain, not mine. But you should refrain from spreading that kind of harmful advice in a forum just because you're to proud to admit that you've been wrong.

However, I'm confident that any sensible reader of this thread (seeking advice on how to implement dialog data validation) will understand nonetheless what's going on and come to the conclusion that DoDataExchange() is the way to go, OnOK being no valid alternative.

For this particular problem, I recommend validating the string in OnOk().

Kuphryn

Quote:

---

Originally posted by kuphryn
For this particular problem, I recommend validating the string in OnOk().

---

Yeah... And for inserting a nail, I recommend a screwdriver. Why? Just don't ask... It's simply my opinion... No need for you to understand it... :rolleyes: