check values on a dialog

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(). :)