Solved

ComboBox-Problem

Posted on 2000-02-23
12
513 Views
Last Modified: 2013-11-20
Hi,there!

Could anybody give me a brief but crucial description how to implement or better how to initialize a ComboBox within the OnInitDialog-section of my program???
Any help welcome. Short Example very welcome.
0
Comment
Question by:BoogieBoy
  • 6
  • 5
12 Comments
 
LVL 32

Expert Comment

by:jhance
ID: 2552272
CComboBox *c = (CComboBox *)GetDlgItem(IDC_MYCOMBOBOX);

c->ResetContent();  // Shouldn't be needed in OnInit... but who knows.

c->AddString("String 1");
c->AddString("String 2");
etc....

SetDlgItemText(IDC_MYCOMBOBOX, "String 1");  // The default text
0
 
LVL 10

Accepted Solution

by:
RONSLOW earned 50 total points
ID: 2552826
Don't use GetDlgItem as it only returns a CWnd (ie not the appropriate class), so calling member functions may stuff up.

Use DDX_Control in you DoDataExchange instead.
ie.

Best way is to use ClassWizard to create a 'control' variable for the combobox (say m_combobox).  If you don't like ClassWizard, the nit is easy to do 'by hand' if you want.

In your OnInitDialog, AFTER the call to the base class (CDialog::OnInitDialog), you can use m_combobox as required.

eg.

BOOL CMyDialob::OnInitDialog() {
  CDialog::OnInitDialog();
  m_combobox.ResetContent();
  m_combobox.AddString("xxx");
  m_combobox.AddString("xxx");
  m_combobox.AddString("zzz");
  m_combobox.SelectString(-1,"yyy");
  m_combobox.SetWindowText("yyy");
  return true;
}

0
 
LVL 32

Expert Comment

by:jhance
ID: 2553744
RONSLOW,

>>Don't use GetDlgItem as it only
>>returns a CWnd (ie not the
>>appropriate class), so calling member
>>functions may stuff up.


BALONEY!!!  

Using:

CComboBox *c = (CComboBox *)GetDlgItem(IDC_MYCOMBOBOX);


is a perfectly acceptable and widely used way to get the CComboBox *.  

Not only does it work perfectly (I don't know what "stuff up" means) but is recommended by Microsoft and all MFC authors that I've read.
0
 
LVL 10

Expert Comment

by:RONSLOW
ID: 2553853
It isn't BALONEY!!! at all !!!

It MAY work perfectly in some cases, dependon the class you are blindly casting too.  But it is NOT perfectly acceptable (but is a widely used bad programming construct).

The reason is ... unless the control has been subclassed (eg DDX_Control) ... GetDlgItem returns a pointer to a CWnd object - not a pointer to a CComboBox object (or whatever).

If you haven't come across this, then you haven't read extensively enough.  And it is certainly NOT recommended by those who know better (and is bad C++ coding)

Look at the C/C++ QA article Sept 96 and Dec 96 by Paul D'Lascia

He explains it well when he says:

>Technically, this code is incorrect because—once again, assuming you haven’t created a CEdit for the ID_EDIT1 control—GetDlgItem returns a CWnd, not a CEdit. You can cast to CEdit all you like, but the object is still a CWnd. In practice, however, this code works because all CEdit::GetSel does is send EM_GETSEL to the window, which, being a true edit control, it responds to. But if GetSel was a virtual function instead of a wrapper, or if it used some data member that was specific to CEdit, this code would crash.

The functions you call just happen to be OK because CComboBox has no additional member data and the particular functions called simply send a message to the window (and as a combo box it understands them).

But in general it is BAD programming practice.
0
 
LVL 32

Expert Comment

by:jhance
ID: 2553880
Of course GetDlgItem returns a CWnd, that's what it's supposed to do.  MFC was carefully design so that all of the window controls are derived from CWnd.  Hence the casting of the CWnd returned from GetDlgItem will always work.

I'll just bet you that DDX_Control uses GetDlgItem() internally......
0
 
LVL 10

Expert Comment

by:RONSLOW
ID: 2553894
You don't seem to get it.

You cannot cast safely from a pointer to a base class object to a pointer to derived.

It is like this

class A {
....
};

class B : public A {
  ...
  void f() {}
}

A objectA;
B* pObjectB = (B*)&objectA;
pObjectB->f();

if f() accesses data or calls virtual function that are NOT defined in A, then CRASH BANG!!!

The same situation with GetDlgItem.

If you haven't subclassed (DDX_Control say), then it creates a temporary CWnd object attached the the control and return a pointer to it.  This object is a CWnd. It doesn't have any of the member data of virtual functions of some class dervied from CWnd .. so you cannot just blindly cast the pointer to something else and expect it to work.

If it does work, it is just sheer luck .. not good planning and certainly not good coding practice.

So, basically, you are WRONG.  The casting will NOT always work !  You'd better fix any code you have written that does this before latter changes make it all fall apart.
0
Maximize Your Threat Intelligence Reporting

Reporting is one of the most important and least talked about aspects of a world-class threat intelligence program. Here’s how to do it right.

 

Author Comment

by:BoogieBoy
ID: 2554296
Well, Guys, chill out, relax!

Thanx for the help and this discussion on the problem. Both answers seem to work but I prefer ronslows solution to the problem because it is very convenient and I actually have to handle ddx functionality. My problem was that I implemented the code for the control-variable BEFORE calling the base-class. Thank u for relief of my headache because I tried a lot of things and the solution is so easy. Just place all m_combobox operations after OnInitDialog() and it works perfect.
0
 
LVL 32

Expert Comment

by:jhance
ID: 2554694
>You don't seem to get it.

>You cannot cast safely from a pointer
>to a base class object to a pointer to
>derived.

You had better re-examine _your_ assumptions!  If what you say is true then COM cannot work!  It, as you say, requires "blindly" casting a pointer from one object to a derived one.

Gee, perhaps COM doesn't really work at all and the world just hasn't noticed it yet!!
0
 
LVL 10

Expert Comment

by:RONSLOW
ID: 2556142
My assumptions are correct.

You need to re-examine yours.  Or re-read your C++ language manuals?

Your comparison to COM is like comparing apples to oragnges.

COM cast from a pointer-to-base-class (that is POINTING TO an object of the DERIVED class) to a pointer-to-derived.  Quite legitimate as long as your "know" that the object being pointer to is of the correct type.

The GetDlgItem business tries to cast from a pointer-to-base-class (that is POINTING TO an object of the BASE class) to a pointer-to-derived.  This is always BAD CODE - and if it "works", that is just "good" luck and NOT good programming.

Consider these two examples:

class A {};
class B : public A {};

A a; B b;
A* pA; B* pB;

// this is the sort of thing COM does
pA = &b;
pB = (B*)pA; // fine because pA actually points to a B object

// this is what GetDlgItem does
pA = &a;
pB = (B*)pA; // BAD, because pA actually points to a A object

It is just straight forward icorrect C++ programming .. it dooesn't have anything to do with MFC / Windows / COM / Price of eggs etc.

If you continue to use that sort of construct (or worse, advise others that it is the right thing to do) then you will end up in trouble.

Eg. say you have a list box on a dialog that is to be a checked list box and you want to set the style, so you say

CCheckedListBox* p = (CCheckedListBox*)GetDlgItem(nId);
p->SetCheckStyle(BS_AUTOCHECKBOX);

ACCESS VIOLATION !!!
CRASH!!!

Why?  Because SetCheckStyle references a member var of CCheckListBox (m_nStyle), and the CWnd object you are pointing to doesn't have that member.

Please .. listen and learn .. for your own sake and that of those you help here and elsewhere.  It is BAD CODING.  PERIOD.
0
 
LVL 32

Expert Comment

by:jhance
ID: 2556289
Let's stop cluttering up Boogie's question with this.

I'm willing to "put up" rather than "shut up" and am posting a new question to continue this discussion.

I think you're wrong on this and contend that using a cast of GetDlgItem is perfectly legal and acceptable for CWnd derived classes.

I'll also explain why the COM example _IS_ relevant to this story....

Watch for it shortly....
0
 
LVL 10

Expert Comment

by:RONSLOW
ID: 2556304
Can't wait .. looking forward to the challenge.

BTW: I KNOW you are wrong on this.
0
 
LVL 10

Expert Comment

by:RONSLOW
ID: 2563305
FYI:

Here is a summary of the evidence form jhances question mentioned above:

jhance's evidence:

He gave three sources (MFC docs, book by Brain and book by Mike Blaz) that showed examples of doing this
- counter argument: they are either examples of special cases that "work" (MFC), or are also incorrect (Brain), or over-generalised (Mike)

He gave examples in MFC source code.
- counter argument: they are either examples of special cases that work, or are were there is no casting, or there is an attached CWnd-derived object.

He gave an example from another doc on C++
- counter argument: was not the same situation and so irrelevant.

my evidence:

a) What is/isn't good, valid and legal C++ (in general and in 'theory').  Given:
  class Base {};
  class Derived : public Base {};
and
  Base b;
  Derived d;
  Base* pB;
  Derived* pD;
These things are fine:
  pB = &b;
  pD = &d;
  pB = &d; // implict upcast
  pD = (Derived*)pB; // ok as pB pointer to Derived object d
But this is not fine
  pB = &b;
  pD = (Derived*)pB; // bad as pB pointer to Base object b;

b) What you can get away with (on most platforms, even though it is theoretically wrong) assuming we have
  pB = &b;
  pD = (Derived*)pB; // bad as pB pointer to Base object b;
this can be ok IFF
*  Derived does not define any memeber variables which are accessed/modified via pD or calls through it
*  Derived does not define any new virtual functions which are called directly of indirectly via pD
*  You do not call any virtaul functions ofBase that are overriden by Derived
*  You do not use runtime type checking, typeof, dynamic_cast etc on pD

Doing any of these things can result in memory corruption, overwrites, illegal addresses etc.

After agreeing to that let's look at GetDlgItem (and the guts of MFC under that).

1) GetDlgItem is declared as returning a CWnd*.  The actual object pointed to by the CWnd* will be either a CWnd or CWnd-derived

2) If the corresponing window is attached to some CWnd-derived (or sometimes CWnd) object, then that attachment is registered in the permanent handle map and GetDlgItem returns a pointer to the actual CWnd-derived object.

3) An entry is put into this map is the object is attached to the window handle, the window is created via the object (Create call) or it is subclassed (often via DDX_Control).

4) If there is no entry in the permanent map for a window handle, then GetDlgItem will create a new temorary CTempWnd and stick that in a temporary map and return a pointer to a CTempWnd.  This is usually the case with dialog control when you don't use DDX_Control and use GetDlgItem instead

5) If you cast the return value of GetDltItem (in case 4 above) to a CWnd-derived class (like CCheckListBox) then you have the bad C++ coding described in the previous message.  You can only "get away with this" if you satify the conditions mentioned before.

6) The only classes that you can cast to (in MFC) and still have 'working' code are (with the current implementation of MFC):
  CEdit, CStatic, CButton, CListBox, CComboBax, CScrollBar
and, through the cast pointer, you should not call any virtual functions of these classes nor of CWnd.

7) Only in these cases (a handful of MFC classes and a subset of their member functions) will the code "work" and only with the current implementation.  One cannot gurantee that there wont be new member vars, virtual functions, runtime class checking etc added in subsequent MFC releases.

8) It is therefore NOT correct to state that you can always cast the return value of GetDlgItem to a pointer to ANY CWnd-derived class (and use that pointer) with safety.  There are cases where this will not work and may be more cases in the future.

9) [the controversial bit] putting this forward as a general techinique without the warnings given is not responsible behaviour, and if such recommendations ARE made, it is reasonable to warn readers that this is NOT a good general technique and only works with certain classes.
 
NO counter arguments have been given to ANY of these, and other experts have agreed.

So, it looks like the correct advice is:

In general, avoid casting the result from GetDlgItem UNLESS you are casting to one of the handful of known classes where this "works" and even then as long as you do not call virtual functions.

BTW: jhance still didn't award me the points despite being correct.  Oh well.
0

Featured Post

IT, Stop Being Called Into Every Meeting

Highfive is so simple that setting up every meeting room takes just minutes and every employee will be able to start or join a call from any room with ease. Never be called into a meeting just to get it started again. This is how video conferencing should work!

Join & Write a Comment

Suggested Solutions

Introduction: Database storage, where is the exe actually on the disc? Playing a game selected randomly (how to generate random numbers).  Error trapping with try..catch to help the code run even if something goes wrong. Continuing from the seve…
Have you tried to learn about Unicode, UTF-8, and multibyte text encoding and all the articles are just too "academic" or too technical? This article aims to make the whole topic easy for just about anyone to understand.
This video will show you how to get GIT to work in Eclipse.   It will walk you through how to install the EGit plugin in eclipse and how to checkout an existing repository.
You have products, that come in variants and want to set different prices for them? Watch this micro tutorial that describes how to configure prices for Magento super attributes. Assigning simple products to configurable: We assigned simple products…

747 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question

Need Help in Real-Time?

Connect with top rated Experts

12 Experts available now in Live!

Get 1:1 Help Now