Prev: writing a service
Next: migration vc6.0 -> vc8.0
From: jaslong on 22 May 2007 17:33 Ok I spent over 24 hours on groups just trying to sub class an MFC CEdit control. All the information I found was correct but I still had to work out a good 30% of the solution myself. Bits of vital - assumed - information was missing. I am posting this in an attempt to stop anyone else having to spend so much time to perform such a simple task. First forget SubClassDlgItem(). This can be used if required, but just obfuscate the code. Ill highlight points of interest and gotchas I found!!!!! Here is the source: This is the custom class .h file which inherits from CEdit. //////////////////////////////////////////////////////////////////////////////////// #pragma once // _CustomEditCTRL class _CustomEditCTRL : public CEdit { DECLARE_DYNAMIC(_CustomEditCTRL) // Changed to match class public: _CustomEditCTRL(); virtual ~_CustomEditCTRL(); // Added manually. COLORREF m_crText; COLORREF m_crBackGnd; CBrush m_brBackGnd; // Add as mere access functions. // Added manually. void _CustomEditCTRL::SetBackColor(COLORREF); // Must be public to call. void _CustomEditCTRL::SetTextColor(COLORREF); protected: afx_msg HBRUSH CtlColor(CDC* pDC, UINT nCtlColor); // Added manually. DECLARE_MESSAGE_MAP() }; This is the custom class cpp file which inherits from CEdit. //////////////////////////////////////////////////////////////////////////////////// // _CustomEditCTRL IMPLEMENT_DYNAMIC(_CustomEditCTRL, CEdit) _CustomEditCTRL::_CustomEditCTRL() { // Added manually. // Set the custom control member variables. MessageBoxA(NULL,"_CustomEditCTRL","",MB_OK); m_crText=RGB(100,0,0); } _CustomEditCTRL::~_CustomEditCTRL() { } // Added manually. HBRUSH _CustomEditCTRL::CtlColor(CDC* pDC, UINT nCtlColor) { MessageBoxA(NULL,"CtlColor","",MB_OK); // Checks that this is called indirectly // never called directly. //set text color pDC->SetTextColor(m_crText); //set the text's background color pDC->SetBkColor(m_crBackGnd); //return the brush used for background this sets control background return (HBRUSH) m_brBackGnd.GetSafeHandle(); } // Added manually. void _CustomEditCTRL::SetBackColor(COLORREF rgb) { MessageBoxA(NULL,"SetBackColor","",MB_OK); // Checks that this is called //set background color ref (used for text's background) m_crBackGnd = rgb; //free brush if (m_brBackGnd.GetSafeHandle()) m_brBackGnd.DeleteObject(); //set brush to new color m_brBackGnd.CreateSolidBrush(rgb); //redraw Invalidate(TRUE); } // Added manually. void _CustomEditCTRL::SetTextColor(COLORREF rgb){ //set text color ref m_crText = rgb; // GOTCHA - added to create background also...can remove. SetBackColor(COLORREF(RGB(255,255,255))); // <- You MUST specify a brush fo 4 the control to work! // If not - no change seems to happen. //redraw Invalidate(TRUE); } BEGIN_MESSAGE_MAP(_CustomEditCTRL, CEdit) // Messages passed around... ON_WM_CTLCOLOR_REFLECT() // GOTCHA - ON_WM_CTLCOLOR will not work use this END_MESSAGE_MAP() // instead. // Added manually. OK we now have a custom class which inherits from a CEdit control... How do we call it? firstly you need to create a member variable for an existing CEdit control which will utilise this custom class. // So in the parent class i.e. the class which hosts the control add this. _CustomEditCTRL m_statusCtrl; // to call this - do this: m_statusCtrl.SetTextColor(COLORREF(RGB(200,0,0))); NOTE: Remember this also calls the background colour function (Cos I told it to as i dont want to change this all the time I want to change the text)...just comment out the SetBackColor(COLORREF(RGB(255,255,255))); BUT specify a default brush after. OK - Rember to add the header file for the _CustomEditCTRL i.e. #include"_CustomEditCTRL .h" where ever you use m_statusCtrl and on the parent of the control. Additional notes: This was tested on vista OK Visual Studios 2005 is pants - No Class Wizards used. PLEASE: This is not a "How great am I" post - This is to try and stop the time wasted I spend trying to get it going....PLEASE comment!!!! This was developed for CPropertyPage with the parent a CPropertySheet (NO Dialog) so maybe the ON_WM_CTLCOLOR windows message would have worked for CDialog parents. I have found some strange posts on Dialog requiring the InitDialog function to be overridden before they recieve and process messages...lol Anyway...If I can help more...pls ask!!!! If I should remove my code as a disgrace to the dark art of MFC then also comment.
From: MrAsm on 22 May 2007 17:56 On 22 May 2007 14:33:35 -0700, jaslong(a)hotmail.com wrote: >Ok I spent over 24 hours on groups just trying to sub class an MFC >CEdit control. All the information I found was correct but I still >had to work out a good 30% of the solution myself. Bits of vital - >assumed - information was missing. > >I am posting this in an attempt to stop anyone else having to spend so >much time to perform such a simple task. Well, there was already an article on MSJ by Paul Di Lascia (reading the web page, it seems it was July 1996 :) explaining how to change the colors of a CEdit control: http://www.microsoft.com/msj/archive/S1DCC.aspx I also belive that CodeProject has something similar... >class _CustomEditCTRL : public CEdit In MFC, the naming convention for classes is CMyClassName (leading 'C', and no underscore...) So it would be better calling your class something like CCustomEditCtrl... (or better: CColorEdit). >public: > _CustomEditCTRL(); > virtual ~_CustomEditCTRL(); > > // Added manually. > COLORREF m_crText; > COLORREF m_crBackGnd; > CBrush m_brBackGnd; I see no reason to expose those data members as 'public'. You should make them 'protected' (or even 'private'). Your class interface is absolutely *not* robust if you expose those data members as public: e.g. you allow the class user to change the background color (m_crBackGnd public member) without updating the background brush. Instead, better making the data members not public (I would make them protected), and expose only public *methods* like Get/SetTextColor and Get/SetBackgroundColor. > void _CustomEditCTRL::SetBackColor(COLORREF); // Must be public to >call. > void _CustomEditCTRL::SetTextColor(COLORREF); > This is ugly C++. There's no reason to repeat the class name in method declaration. void SetBackColor( COLORREF crBackColor ); void SetTextColor... would be just fine. >_CustomEditCTRL::_CustomEditCTRL() >{ > // Added manually. > // Set the custom control member variables. > MessageBoxA(NULL,"_CustomEditCTRL","",MB_OK); I don't like those MessageBox calls (here and in other part of the code you showed). If you want some form of feed-back you may use TRACE, or at least if you really like the MessageBox'es (why???) use some form of preprocessor so we can turn them on/off via a simple #define, e.g. #ifdef LOG_USING_MESSAGEBOX MessageBoxA... #endif > m_crText=RGB(100,0,0); Well, you'd better init also the other class fields, like background color and background brush. >HBRUSH _CustomEditCTRL::CtlColor(CDC* pDC, UINT nCtlColor) >{ > MessageBoxA(NULL,"CtlColor","",MB_OK); // Checks that this is Bad MessageBox again. >// Added manually. >void _CustomEditCTRL::SetTextColor(COLORREF rgb){ > > //set text color ref > m_crText = rgb; > // >GOTCHA - added to create background also...can remove. > SetBackColor(COLORREF(RGB(255,255,255))); // <- You MUST specify a >brush fo 4 the control to work! No: this is wrong. Why are you forcing a white background color when the user changes the text color? If the user wants a white background color, he/she must call SetBackColor. Why are you calling that method in SetTextColor body?? Moreover, I don't like the COLORREF(RGB(...)) syntax; it's baroque and redundant: RGB already returns a COLORREF, so SetBackColor(RGB(...)) is just fine. >PLEASE: This is not a "How great am I" post - This is to try and stop >the time wasted I spend trying to get it going....PLEASE comment!!!! Done, my 2 cents :) MrAsm
From: Scott McPhillips [MVP] on 22 May 2007 20:18 jaslong(a)hotmail.com wrote: > Ok I spent over 24 hours on groups just trying to sub class an MFC > CEdit control. All the information I found was correct but I still > had to work out a good 30% of the solution myself. Bits of vital - > assumed - information was missing. > > I am posting this in an attempt to stop anyone else having to spend so > much time to perform such a simple task. > > First forget SubClassDlgItem(). This can be used if required, but > just obfuscate the code. >...<code> Just to further your education... You have a terminology problem here and a misunderstanding. The term "subclass" has a very specific meaning in Windows programming that is not related to C++ classes. In Windows, subclassing is the interception of messages that would normally go to a window. If you do not subclass the control your class will not receive any messages. One way to subclass the control is SubclassDlgItem. The other way is the DDX_Control call that is normally added by the MFC wizard. Your post implies that you did neither, so you have some misunderstanding of what you actually did to get it to work! -- Scott McPhillips [MVP VC++]
From: Joseph M. Newcomer on 23 May 2007 01:24 On 22 May 2007 14:33:35 -0700, jaslong(a)hotmail.com wrote: >Ok I spent over 24 hours on groups just trying to sub class an MFC >CEdit control. All the information I found was correct but I still >had to work out a good 30% of the solution myself. Bits of vital - >assumed - information was missing. > >I am posting this in an attempt to stop anyone else having to spend so >much time to perform such a simple task. > >First forget SubClassDlgItem(). This can be used if required, but >just obfuscate the code. > >Ill highlight points of interest and gotchas I found!!!!! > >Here is the source: > >This is the custom class .h file which inherits from CEdit. >//////////////////////////////////////////////////////////////////////////////////// > >#pragma once > > >// _CustomEditCTRL > >class _CustomEditCTRL : public CEdit **** Putting _ in front is a Really Bad Idea. Avoid doing this. It violates a lot of programming standards, such as the C specification that reserves symbols starting with _ for vendor extensions. ***** >{ > DECLARE_DYNAMIC(_CustomEditCTRL) // Changed to match class > >public: > _CustomEditCTRL(); > virtual ~_CustomEditCTRL(); > > // Added manually. ***** Stop spending time writing "Added manually" as a comment. It wastes space and conveys nothing useful, since nearly ALL code is "added manually". Only a few things are added by the wizards. protected: ****** > COLORREF m_crText; > COLORREF m_crBackGnd; > CBrush m_brBackGnd; **** I see no reason to make any of the three above lines public. You have operations SetBackColor and SetTextColor, so these variables should all be protected. public: **** > > // Add as mere access >functions. // Added manually. **** Lose the "added manually" comments. ***** > void _CustomEditCTRL::SetBackColor(COLORREF); // Must be public to >call. ***** THe comment is silly, since that is part of the C++ language definition. It is the only possible way C++ can work! ***** > void _CustomEditCTRL::SetTextColor(COLORREF); > >protected: > > afx_msg HBRUSH CtlColor(CDC* pDC, UINT nCtlColor); // Added >manually. > DECLARE_MESSAGE_MAP() >}; > > > >This is the custom class cpp file which inherits from CEdit. >//////////////////////////////////////////////////////////////////////////////////// > > >// _CustomEditCTRL > >IMPLEMENT_DYNAMIC(_CustomEditCTRL, CEdit) > >_CustomEditCTRL::_CustomEditCTRL() >{ > // Added manually. ***** Lose the comment. ***** > // Set the custom control member variables. ***** Lose this one also. That is what constructors are for, and saying it redundantly simply wastes time and space and adds meaningless clutter to the code ***** > MessageBoxA(NULL,"_CustomEditCTRL","",MB_OK); **** Have you considered using the TRACE function to write output? A MessageBox (why MessageBoxA? Silly!) should NEVER have a NULL first parameter, because then it means it can pop up UNDER your app! AfxMessageBox(_T("_CustomEditCTRL()")); would have been easier to write, and would get it right. But better still would be TRACE(_T("_CustomEditCTRL::CustomEditCTRL()\n")); since it works when MessageBox would not. **** > m_crText=RGB(100,0,0); **** You do not create the brush here. Why not? What is the random color choice here? Why not m_crText = ::GetSysColor(COLOR_WINDOWTEXT); as the default? m_crBackGnd = ::GetSysColor(COLOR_WINDOW); m_brBackGnd.CreateSolidBrush(m_crBackGnd); The purpose of a constructor is to initialize everything that needs to be initialized. You failed to do that. **** >} > >_CustomEditCTRL::~_CustomEditCTRL() >{ >} > > >// Added manually. >HBRUSH _CustomEditCTRL::CtlColor(CDC* pDC, UINT nCtlColor) >{ > MessageBoxA(NULL,"CtlColor","",MB_OK); // Checks that this is >called indirectly ***** Use TRACE as indicated above. ***** > // >never called directly. > //set text color > pDC->SetTextColor(m_crText); > > //set the text's background color > pDC->SetBkColor(m_crBackGnd); > > //return the brush used for background this sets control background > return (HBRUSH) m_brBackGnd.GetSafeHandle(); **** Since you do not create a brush in the constructor, this will fail, and your text color and BkColor will be ignored until such time as you set a brush. Actually, you don't need GetSafeHandle() here. Just do return m_brBackGnd; since it will implicitly apply the (HBRUSH) operator to the brush object. ***** >} > >// Added manually. >void _CustomEditCTRL::SetBackColor(COLORREF rgb) >{ > > MessageBoxA(NULL,"SetBackColor","",MB_OK); // Checks >that this is called **** Use TRACE as indicated above ***** > //set background color ref (used for text's background) > m_crBackGnd = rgb; > > //free brush > if (m_brBackGnd.GetSafeHandle()) > m_brBackGnd.DeleteObject(); **** You don't need to test the GetSafeHandle to do the DeleteObject. If the object handle is NULL, DeleteObject does nothing. ***** > //set brush to new color > m_brBackGnd.CreateSolidBrush(rgb); > > //redraw > Invalidate(TRUE); ***** Note that you DO need to check the GetSafeHwnd() here: if(GetSafeHwnd() != NULL) Invalidate(); If you call this method before the window is bound (which should be a valid thing to do) then Invalidate() will take an assertion error. ***** >} > >// Added manually. **** You don't need to keep saying "added manally" since 90% of your code will have this comment **** >void _CustomEditCTRL::SetTextColor(COLORREF rgb){ > > //set text color ref > m_crText = rgb; > // >GOTCHA - added to create background also...can remove. > SetBackColor(COLORREF(RGB(255,255,255))); // <- You MUST specify a >brush fo 4 the control to work! > // ***** This isn't a "gotcha", it is screamingly obvious. You can't use an uninitialized variable, and you had failed to initialize it. What's the "gotcha"? By the way, your default constructor should do SetBackColor(::GetSysColor(COLOR_WINDOW)); why do you think RGB(255,255,255) is right? (Hint: it isn't) ***** >If not - no change seems to happen. ***** Yes, that is obvious and is the expected behavior. Or anything else that is not correct is the expected behavior. Note that the OnCtlColor handler tells you that you must return a valid HBRUSH. NULL is not a valid HBRUSH, so the behavior is unspecified, but observation shows that all settings are discarded. This is what I meant by an uninitialized variable. If you don't follow the specs, you can expect to have odd behavior. ***** > //redraw > Invalidate(TRUE); **** The //redraw comment is irrelevant, since that's what Invalidate() does. This MUST be written as if(GetSafeHwnd() != NULL) Invalidate(); **** >} > > >BEGIN_MESSAGE_MAP(_CustomEditCTRL, CEdit) // Messages passed >around... > ON_WM_CTLCOLOR_REFLECT() // GOTCHA - ON_WM_CTLCOLOR will not >work use this **** That is not a "gotcha". If you want a reflected handler you would use the Properties list to add a =WM_CTLCOLOR handler. OF COURSE "ON_WM_CTLCOLOR" won't work; it is used to handle notifications from a CHILD control, and this edit control has no child control! ***** >END_MESSAGE_MAP() // >instead. // Added manually. **** Lose the Added manually comment **** > > >OK we now have a custom class which inherits from a CEdit control... >How do we call it? > >firstly you need to create a member variable for an existing CEdit >control which will utilise this custom class. > >// So in the parent class i.e. the class which hosts the control add >this. > >_CustomEditCTRL m_statusCtrl; > > >// to call this - do this: ***** No, that is an incorrect statement. You don't "call" m_statusCtrl, you call METHODS of the class. ***** >m_statusCtrl.SetTextColor(COLORREF(RGB(200,0,0))); ***** Well, note that you did NOT check to see if you have a valid window before calling Invalidate(). Yet here you are calling SetTextColor with no evidence that the control has been bound. You should have this line in your DoDataExchange handler (and use the ClassWizard to do this if you don' t know how to do it) DDX_Control(pDX, IDC_WHATEVER, m_statusCtrl); so without that line, the SetTextColor will ASSERT fail on the Invalidate() line. You DO have a DoDataExchange handler in your dialog....? ***** > >NOTE: Remember this also calls the background colour function (Cos I >told it to as i dont want to change this all the time I want to change >the text)...just comment out the >SetBackColor(COLORREF(RGB(255,255,255))); BUT specify a default brush >after. ***** DO NOT assume that the color of a window is RGB(255,255,255). The correct color of a window is obtained by ::GetSysColor(COLOR_WINDOW) If you had initialized your brush in your constructor this discussion would not be necessary, and therefore you should initialize your brush in your constructor. ***** > >OK - Rember to add the header file for the _CustomEditCTRL i.e. >#include"_CustomEditCTRL .h" where ever you use m_statusCtrl and on >the parent of the control. ***** Yes, that is kind of obvious, elementary C programming style. You have to define a symbol before it is used. ***** > > >Additional notes: > >This was tested on vista OK >Visual Studios 2005 is pants - No Class Wizards used. ***** Why not? Do you enjoy being miserable, wasting time, and generating code that doesn't work right the first time? After more than a decade of MFC programming, I can get away without using the wizards, but prefer to use them. As a relative beginner, you should use every tool you have available to simplify your life. Rolling your own by hand is a sure way to waste massive amounts of time. **** > > >PLEASE: This is not a "How great am I" post - This is to try and stop >the time wasted I spend trying to get it going....PLEASE comment!!!! > >This was developed for CPropertyPage with the parent a CPropertySheet **** Sounds fine. **** >(NO Dialog) ***** That statement makes no sense. The CPropertyPage *IS* a dialog. ***** >so maybe the ON_WM_CTLCOLOR windows message would have >worked for CDialog parents. **** A CPropertyPage IS a CDialog! Go look at the documentation for CPropertyPage! ***** >I have found some strange posts on Dialog >requiring the InitDialog function to be overridden before they recieve >and process messages...lol ***** Well, if you have to initialize anything in the dialog, that is common. You certainly have to have a DoDataExchange method, which if you use the wizards to create your classes comes free, but if you are rolling your own, you have to know how to write. Frankly, writing your own CWnd-derived classes without wizards is largely a waste of time, and should only be attempted by experts. Beginners have no chance of getting it right. I rarely write a class by hand, even with over 12 years of writing MFC code. ***** > >Anyway...If I can help more...pls ask!!!! >If I should remove my code as a disgrace to the dark art of MFC then >also comment. ***** Essentially, all you did was forget to initialize a variable and try to use it before it was initialized, use the wrong method to handle a notification in a child control, and fail to test for the existence of a window before operating on it. These are pretty much elementary C programming or really basic MFC. So having worked them out, you are unlikely to forget them. In the future, use the tools. It will save you massive amounts of time. I find very few excuses for writing classes "by hand", and none at all for beginners. joe ***** Joseph M. Newcomer [MVP] email: newcomer(a)flounder.com Web: http://www.flounder.com MVP Tips: http://www.flounder.com/mvp_tips.htm
From: jaslong on 23 May 2007 05:12 Great comments guys....much appriciated. I have changed my code accordingly to reflect these and also taken onboard understanding issues raised.
|
Pages: 1 Prev: writing a service Next: migration vc6.0 -> vc8.0 |