From: jaslong on
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
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
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
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
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