Re: MessageBox hangs app. AfxMessageBox
[posted and mailed]
Joseph M. Newcomer <newcomer@flounder.com> wrote in
news:ntbaj3t9vdi2sab5f2ouodnnhm1vpetm9u@4ax.com:
On Fri, 09 Nov 2007 20:35:10 GMT, JC <elvis_is_king@bellsouth.net>
wrote:
Subject: Message box hangs application
From: JC <elvis_is_king@bellsouth.net>
Newsgroups: microsoft.public.vstudio.development
- Apologies for posting in 2 groups separately.
I also posted this on microsoft.public.vstudio.development
Will properly cross-post next time. I don't use newsgroups a lot and
am just getting the hang of this Xnews reader.
- Apologies for posting in 2 groups separately.
=====================================================================
=====================================================================
=====================================================================
=====================================================================
I am an embedded programmer that is new to Windows programming. So,
that makes me a newbie to Windows (other than a little maintenance
here and there).
I have created a simple application in VS2005. It is a dialog app (not
SDI or MDI). In the application code a CDialog is created; this was
put there by VS2005 code generation.
In the application code a CDialog is created (as mentioned
previously). Then I added a property sheet from the CDialog class.
Then I added property pages from the CPropertySheet class.
When I try to put up an AfxMessageBox or MessageBox, the app. hangs.
I set all dialogs and sheets as "'child" in the dialog editor.
Any ideas? Any help would be greatly appreciated.
--
Regards,
JC
Here are the code excerpts.
========================================================
BOOL CTestCaseApp::InitInstance()
{
... Other wizard generated code...
CTestCaseDlg dlg;
m_pMainWnd = &dlg;
INT_PTR nResponse = dlg.DoModal();
return FALSE;
}
BOOL CTestCaseDlg::OnInitDialog()
{
CDialog::OnInitDialog(); // wizard generated
... Other wizard generated code...
m_pPSheetCase = new CPSheetCase(m_sWinTitle, (CWnd*)this, 0);
m_pPSheetCase->Create((CWnd*)this, dwStyle);
****
There is no reason to cast this to a CWnd*; get rid of the cast.
****
**==**==**
OK! Got it.
**==**==**
m_pPSheetCase->ShowWindow(SW_SHOWNORMAL);
m_pPSheetCase->SetFocus();
m_pbIDOK = (CButton*)GetDlgItem(IDOK);
****
DO NOT USE GetDlgItem!!!!! This code is potentially wrong, because
GetDlgItem creates a temporary object and the value must not be saved
across message pump invocations. But when you write GetDlgItem, the
very first thought you should have is "I am making a serious mistake".
After some years of MFC programming, you can use it if you know
exactly what you are doing, but you never, ever want to save it in
other than a local variable. I write two to four GetDlgItem calls per
year. You have nearly written more than that in one subroutine.
Right click on the control and do "Add Variable" to get a control
variable, and then you don't need to use ->; you would just write
m_pblDOK.EnableWindow(FALSE);
****
**==**==**
I should have put a little more disclaimer in my original post. I have
looked at your site occasionally for over a year even though I didn't
really do any Windows programming except for a little maintenance on some
apps. that violate most of the discipline you explain. I did actually
intend to remove that GetDlgItem(). I read your article on that a long
time ago and am a Newcomer convert. By the way, one of the apps that I
had to maintain calls GetDlgItem for EVERYTHING. Out of maybe 100
controls, not one has a variable created for it. Of course, I am not
alotted the time to fix it and as a newbie, I may break more than I am
able to fix!
**==**==**
m_pbIDOK->EnableWindow(0);
****
I looked at the documentation of EnableWindow, and it clearly states
the parameter is BOOL. Why are you using an integer value here?
FALSE would work, but 0 makes no sense. Never use an integer for a
BOOL argument.
****
**==**==**
OK! Got it.
**==**==**
m_pbIDOK->ShowWindow(0);
****
I looked at the documentation of ShowWindow, and not one of the ten
documented values you can use for the argument is 0. I have no idea
what 0 means. You might use SW_SHOW, or SW_HIDE, or any of the other
values, but 0 is nothing but meaningless noise. What possible meaning
could this have? I don't know what this will do. And I shouldn't
have to. Use only documented values. 0 is not a documented value and
must not be used.
Note that I would never enable or disable any control in any dialog in
the OnInitDialog. I establish the state and call a single
updateControls() handler whose responsibility is to
enable/disable/show/hide EVERY control that has changeable state.
Controls are never enabled or disabled anywhere else. Otherwise you
end up with an unmanageable mess. The last call in my OnInitDialog,
right before the return TRUE, is a call on updateControls().
****
**==**==**
Very good advice. MUCH APPRECIATED! It seems that I do remember that you
have a "tips" article about this, I will go check.
**==**==**
/*--------------------------------------------------------------
By default, from the VS project wizard, the "Close button" which
is the 'X' in the upper-right corner of the dialog does not work
with the IDCANCEL button disabled. To fix it, WM_ON_CLOSE() was
added to the message map for this file. Also, the overridden
funcion OnClose() had to be created. Now the 'X' calls OnClose()
in this cpp file. JAC
--------------------------------------------------------------*/
m_pbIDCANCEL = (CButton*)GetDlgItem(IDCANCEL);
m_pbIDCANCEL->EnableWindow(0);
m_pbIDCANCEL->ShowWindow(0);
****
Ditto.
****
**==**==**
Ditto back
**==**==**
m_pbID_HELP = (CButton*)GetDlgItem(ID_HELP);
m_pbID_HELP->EnableWindow(0);
m_pbID_HELP->ShowWindow(0);
****
Ditto
****
**==**==**
Ditto back
**==**==**
return TRUE; // return TRUE unless you set the focus to a
control
}
CPSheetCase::CPSheetCase(LPCTSTR pszCaption, CWnd* pParentWnd, UINT
iSelectPage)
:CPropertySheet(pszCaption, pParentWnd, iSelectPage)
{
m_pOsInfo = new (OSVERSIONINFO);
m_pOsInfo->dwOSVersionInfoSize = sizeof(OSVERSIONINFO);
GetVersionEx(m_pOsInfo);
****
Why are you allocating a new object and storing it in a pointer to an
object? If you want an OSVERSIONINFO, you would declare a member
variable
OSVERSIONINFO m_pOsInfo;
There is no justification for using a pointer variable or allocating
something using new. Another good rule in C++ programming is the
reallization that if you write 'new' you had better have a very good
reason for writing it, and in this case you do not.
m_pOsInfo.dwOSVersionInfoSize = sizeof(OSVERSIONINFO);
GetVersionEx(&m_pOsInfo);
****
**==**==**
I cut and pasted this from one of those other apps that I need to
maintain. I saw that the other application checked for OS level so I
shouls too. I will fix it. Just because I need to check OS level doesn't
mean I need to do it incorrectly!
**==**==**
//Check if XP Operating System
bIsWindowsXPorLater = ((m_pOsInfo->dwMajorVersion >5 ) ||
( (m_pOsInfo->dwMajorVersion ==5 ) &&
(m_pOsInfo->dwMinorVersion >=1 ) ) );
m_pPPage1 = NULL;
****
Why are you assigning NULL to a variable that the next statement
assigns to? Delete these assignments of NULL!
****
**==**==**
My thought here was just setting up a convention to insure invalid
pointers were NULL. I may not always be setting the pionter to its object
right after I set it to NULL. I thought maybe I will have to move the
"new CPropertyPage" somewhere else in code eventually and the
constructore will just guarantee they are at a known state. But, your are
right. They are currently not needed. I will remove them.
**==**==**
m_pPPage1 = new CPPageCase1;
AddPage(m_pPPage1);
m_pPPage2 = NULL;
m_pPPage2 = new CPPageCase2;
AddPage(m_pPPage2);
m_pPPage3 = NULL;
m_pPPage3 = new CPPageCase3;
AddPage(m_pPPage3);
m_pPPage4 = NULL;
m_pPPage4 = new CPPageCase4;
AddPage(m_pPPage4);
}
void CPPageCase1::OnBnClickedPpage()
{
CPropertySheet* pParent;
pParent = static_cast<CPropertySheet*>(GetParent());
// pParent->GetParent()->MessageBox("Shit");
// pParent->MessageBox("Msg1");
// pParent = ::GetParent((HWND)this);
****
OK, what is going on here? This looks like it responds to a button
clicked on property sheet 1. If you want to pop up a MessageBox, why
are you not doing AfxMessageBox? The parent of the message box should
be the dialog which is the property page. I see no reason the parent
should be the property sheet. I have no idea what the consequences
would be, but overall, if you are seeing bad behavior from a
MessageBox, the first thing to do is make sure that the MessageBox
call is correct. If a property page wants to pop up a message box,
then the property page should be the parent; this has to do with how
modal dialogs disable windows associated with them.
Generally, it is considered Best Practice to use AfxMessageBox rather
than CWnd::MessageBox. This is because the standards are that a
MessageBox should have the app name. There is nothing more dismaying
than to come to a computer and see a MessageBox that says
-------------------------
| Error |
______________
| You have a ... |
| [OK] |
______________
It doesn't matter what the text of the message is; WHICH OF MY 35
RUNNING APPS ISSUED IT? This is why the app name must be in the
caption, and AfxMessageBox takes care of that.
Go browse my MVP Tips site. In particular, the essays such as
"Avoding GetDlgItem", "Avoiding UpdateData", "Dialog control
management" and "The n habits of highly defective Windows programs"
are recommended.
joe
****
**==**==**
Yeah, I was just trying to respond to the OK button on a ProprtySheet and
put up a message box. Just trying stuff and learning. I did originally
try AfxMessageBox. You will see that it is currently commented out as I
was grasping at straws and just trying all kinds of crap to see if I
could it to work. I tried getting a pointer to the PropertySheet and
having the sheet put it up, that didn't work. I tried MessageBox and
AfxMessageBox. They all cause a hang. I can call AfxMessageBox from the
PropertySheet in the constructor and it works. I just can't get the thing
to work from the PropertyPage. A coworker mentioned that I may have to
put "Reflect()" in to get the message to propogate correctly.
**==**==**
// AfxMessageBox("Msg2",MB_TOPMOST | MB_SETFOREGROUND | MB_OK);
}
========================================================
Joseph M. Newcomer [MVP]
email: newcomer@flounder.com
Web: http://www.flounder.com
MVP Tips: http://www.flounder.com/mvp_tips.htm
**==**==**
================================================================
Thanks for all the support to a newbie on this.
I am not sure what proper newsgroup etiquette is. Do I respond to
all individually or address all in one response. I am going to
respond to all individually for now.
Also, Xnews give the options:
"Followup to newsgroup"
"Reply by Mail"
"Follwup and Mail Reply"
I will use "Followup and Mail Reply" to see how that goes. Please
let me know if that is incorrect.
================================================================
I got the same Xnews error as the others "10049 Address Not Found" so I
just replied to the newgroup.
--
Regards,
JC
**==**==**