duplicated member variables in derived classes
-
CArray is X| There is std::vector, and since VC6 it is actually working. Use virtuality when you need it. Especially the lack of virtual d'tors would get you a hard time over here... And with providing a virtual d'tor, you get rid of these annoying effects. The lack of myType-Initialisation in A was deliberately?!?
class A
{
public:
TClassType myType;
A() {}
virtual ~A() {}
};class B : public A
{
TClassType myType;
public:
B() : myType(TYPEB) {}
virtual ~B() {}
};class C : public A
{
public:
TClassType myType;
C() : myType(TYPEC) {}
virtual ~C() {}
};
"We trained hard, but it seemed that every time we were beginning to form up into teams we would be reorganised. I was to learn later in life that we tend to meet any new situation by reorganising: and a wonderful method it can be for creating the illusion of progress, while producing confusion, inefficiency and demoralisation." -- Caius Petronius, Roman Consul, 66 A.D.
Use of CArray vs. using std::vector has nothing to do with the point I was trying to make. Even with the virtual destructor and with an initialization of myType in the A constructor, the effect would still be the same in the B and C case. Suppose we were iterating over all of the elements in myArray and examining the value of myType hoping to figure out what class type we would need to cast the pointer returned in the call to GetAt. myArray.Add(new A); myArray.Add(new B); myArray.Add(new C); for (int i=0;imyType) { case TYPEA: DoSomethingWithTypeA(pA); break; case TYPEB: DoSomethingWithTypeB((B*)ptr); break; case TYPEC: DoSomethingWithTypeC((C*)ptr); break; } } You would never see a call to DoSomethingWithTypeB or DoSomethingWithTypeC, even though element 1 is type B and element 2 is type C. I agree with your comment of using virtual functions and/or destructors. A better way to do this, would be to use a virtual function to do whatever would be done in the respective DoSomethingWith-- calls. Like this: // FooTest.cpp : Defines the entry point for the console application. // #include "stdafx.h" #include class A { public: A() {}; virtual ~A(){}; virtual void DoSomething(); }; class B : public A { public: B() {} virtual ~B() {}; virtual void DoSomething(); }; class C : public A { public: C() {} virtual ~C() {}; virtual void DoSomething(); }; void A::DoSomething() { //Do something with type A } void B::DoSomething() { //Do something with type B } void C::DoSomething() { //Do something with type C } int _tmain(int argc, _TCHAR* argv[]) { CArray myArray; myArray.Add(new A); myArray.Add(new B); myArray.Add(new C); for (int i=0;iDoSomething(); } //Get rid of the memory leak so as to avoid getting a bunch of bad style comments for (int i=0;i
-
Consider the following simple Win32 console app. It defines 3 classes A, B, and C. The classes B and C derive from class A. We create a CArray of class A pointers. Since B and C are derived from A, we can add pointers to B and C to the CArray. On the lines with "LINE A", "LINE B", and "LINE C" comments, what would the value of pA->myType be in each case? Hint, it won't be TYPEA, TYPEB, or TYPEC! To test this app, set breakpoints at each of the 3 lines with the calls to GetAt, and view the value of pA->myType in the debugger.
// FooTest.cpp : Defines the entry point for the console application.
//#include "stdafx.h"
#include "FooTest.h"
#include <afxtempl.h>#ifdef _DEBUG
#define new DEBUG_NEW
#endif// The one and only application object
CWinApp theApp;
using namespace std;
enum TClassType {TYPEA, TYPEB, TYPEC};
class A
{
public:
A() {};
~A();
TClassType myType;
};class B : public A
{
public:
B() { myType = TYPEB; }
~B();
TClassType myType;
};class C : public A
{
public:
C() { myType = TYPEC; }
~C();
TClassType myType;
};int _tmain(int argc, TCHAR* argv[], TCHAR* envp[])
{
int nRetCode = 0;
CArray<A*> myArray;
A* pA;myArray.Add(new A); myArray.Add(new B); myArray.Add(new C); pA = myArray.GetAt(0); //LINE A pA = myArray.GetAt(1); //LINE B pA = myArray.GetAt(2); //LINE C // initialize MFC and print and error on failure if (!AfxWinInit(::GetModuleHandle(NULL), NULL, ::GetCommandLine(), 0)) { // TODO: change error code to suit your needs \_tprintf(\_T("Fatal Error: MFC initialization failed\\n")); nRetCode = 1; } else { // TODO: code your application's behavior here. } return nRetCode;}
That is an issue, but it is also, I think, a bad design of the class hierarchy. :)
If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler. -- Alfonso the Wise, 13th Century King of Castile.
-
Use of CArray vs. using std::vector has nothing to do with the point I was trying to make. Even with the virtual destructor and with an initialization of myType in the A constructor, the effect would still be the same in the B and C case. Suppose we were iterating over all of the elements in myArray and examining the value of myType hoping to figure out what class type we would need to cast the pointer returned in the call to GetAt. myArray.Add(new A); myArray.Add(new B); myArray.Add(new C); for (int i=0;imyType) { case TYPEA: DoSomethingWithTypeA(pA); break; case TYPEB: DoSomethingWithTypeB((B*)ptr); break; case TYPEC: DoSomethingWithTypeC((C*)ptr); break; } } You would never see a call to DoSomethingWithTypeB or DoSomethingWithTypeC, even though element 1 is type B and element 2 is type C. I agree with your comment of using virtual functions and/or destructors. A better way to do this, would be to use a virtual function to do whatever would be done in the respective DoSomethingWith-- calls. Like this: // FooTest.cpp : Defines the entry point for the console application. // #include "stdafx.h" #include class A { public: A() {}; virtual ~A(){}; virtual void DoSomething(); }; class B : public A { public: B() {} virtual ~B() {}; virtual void DoSomething(); }; class C : public A { public: C() {} virtual ~C() {}; virtual void DoSomething(); }; void A::DoSomething() { //Do something with type A } void B::DoSomething() { //Do something with type B } void C::DoSomething() { //Do something with type C } int _tmain(int argc, _TCHAR* argv[]) { CArray myArray; myArray.Add(new A); myArray.Add(new B); myArray.Add(new C); for (int i=0;iDoSomething(); } //Get rid of the memory leak so as to avoid getting a bunch of bad style comments for (int i=0;i
bolivar123 wrote:
Use of CArray vs. using std::vector has nothing to do with the point I was trying to make.
Granted, but is is my habit to object against
CArraywhenever I see its use.:rolleyes: It was created with the early MFCs. Out of dire need when the MS-C++ was so fundamentally broken as being incapable of compiling any STL. There is no need to use the afxtempl.h-collections in new software! You are right withfor (int i=0;imyType)
{
case TYPEA:
DoSomethingWithTypeA(pA);
break;
case TYPEB:
DoSomethingWithTypeB((B*)ptr);
break;
case TYPEC:
DoSomethingWithTypeC((C*)ptr);
break;
}
}pA->myTypewill always beTYPEA(or undefined, with your unmodified code from the first posting). I had been misled by the debugger (VC2003), which is capable of showing the virtual variable (TYPEB, TYPEC) when the mouse is hovering overpA. When hovering overpA->myType, the correct value is shown, though. This whole issue is one of the confusing points of the C++ typesystem, but not a subtle bug, I think. You ordered a A*, so you got an A*.
"We trained hard, but it seemed that every time we were beginning to form up into teams we would be reorganised. I was to learn later in life that we tend to meet any new situation by reorganising: and a wonderful method it can be for creating the illusion of progress, while producing confusion, inefficiency and demoralisation." -- Caius Petronius, Roman Consul, 66 A.D.
-
bolivar123 wrote:
Use of CArray vs. using std::vector has nothing to do with the point I was trying to make.
Granted, but is is my habit to object against
CArraywhenever I see its use.:rolleyes: It was created with the early MFCs. Out of dire need when the MS-C++ was so fundamentally broken as being incapable of compiling any STL. There is no need to use the afxtempl.h-collections in new software! You are right withfor (int i=0;imyType)
{
case TYPEA:
DoSomethingWithTypeA(pA);
break;
case TYPEB:
DoSomethingWithTypeB((B*)ptr);
break;
case TYPEC:
DoSomethingWithTypeC((C*)ptr);
break;
}
}pA->myTypewill always beTYPEA(or undefined, with your unmodified code from the first posting). I had been misled by the debugger (VC2003), which is capable of showing the virtual variable (TYPEB, TYPEC) when the mouse is hovering overpA. When hovering overpA->myType, the correct value is shown, though. This whole issue is one of the confusing points of the C++ typesystem, but not a subtle bug, I think. You ordered a A*, so you got an A*.
"We trained hard, but it seemed that every time we were beginning to form up into teams we would be reorganised. I was to learn later in life that we tend to meet any new situation by reorganising: and a wonderful method it can be for creating the illusion of progress, while producing confusion, inefficiency and demoralisation." -- Caius Petronius, Roman Consul, 66 A.D.
Previous versions of MFC had a CArray class and different variants CStringArray, CObArray, etc. not the CArray template.
-
That is an issue, but it is also, I think, a bad design of the class hierarchy. :)
If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler. -- Alfonso the Wise, 13th Century King of Castile.
This actually happened in one of our applications. We have a lot of code that has been ported and refactored from C to C++. So some of the classes we now have, were once C structs. In one case the myType member was duplicated. This is how I came across this little problem.
-
This actually happened in one of our applications. We have a lot of code that has been ported and refactored from C to C++. So some of the classes we now have, were once C structs. In one case the myType member was duplicated. This is how I came across this little problem.
So you have to take into account a little re-design step in your refactoring process... :)
If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler. -- Alfonso the Wise, 13th Century King of Castile.
-
Previous versions of MFC had a CArray class and different variants CStringArray, CObArray, etc. not the CArray template.
bolivar123 wrote:
Previous versions of MFC had a CArray class and different variants CStringArray, CObArray, etc. not the CArray template.
Even so, MFC collection classes are considered deprecated within MS VC++ team. See this thread[^]: The MFC collection classes are only there for backwards compatibility. C++ has a standard for collection classes and that is the Standards C++ Library. There is no technical drawback for using any of the standard library in an MFC application. We do not plan on making significant changes in this area. Ronald Laeremans Acting Product Unit Manager Vsual C++ Team
-
bolivar123 wrote:
Previous versions of MFC had a CArray class and different variants CStringArray, CObArray, etc. not the CArray template.
Even so, MFC collection classes are considered deprecated within MS VC++ team. See this thread[^]: The MFC collection classes are only there for backwards compatibility. C++ has a standard for collection classes and that is the Standards C++ Library. There is no technical drawback for using any of the standard library in an MFC application. We do not plan on making significant changes in this area. Ronald Laeremans Acting Product Unit Manager Vsual C++ Team
Oh goodie! If they don't plan on changing CArray, I'll continue using it. I just hate it when they go changing a good thing. Like the changes in CString from MFC 6 to MFC 7. Geez, I didn't think the original post would tangent to a debate on which is better......
-
bolivar123 wrote:
Use of CArray vs. using std::vector has nothing to do with the point I was trying to make.
Granted, but is is my habit to object against
CArraywhenever I see its use.:rolleyes: It was created with the early MFCs. Out of dire need when the MS-C++ was so fundamentally broken as being incapable of compiling any STL. There is no need to use the afxtempl.h-collections in new software! You are right withfor (int i=0;imyType)
{
case TYPEA:
DoSomethingWithTypeA(pA);
break;
case TYPEB:
DoSomethingWithTypeB((B*)ptr);
break;
case TYPEC:
DoSomethingWithTypeC((C*)ptr);
break;
}
}pA->myTypewill always beTYPEA(or undefined, with your unmodified code from the first posting). I had been misled by the debugger (VC2003), which is capable of showing the virtual variable (TYPEB, TYPEC) when the mouse is hovering overpA. When hovering overpA->myType, the correct value is shown, though. This whole issue is one of the confusing points of the C++ typesystem, but not a subtle bug, I think. You ordered a A*, so you got an A*.
"We trained hard, but it seemed that every time we were beginning to form up into teams we would be reorganised. I was to learn later in life that we tend to meet any new situation by reorganising: and a wonderful method it can be for creating the illusion of progress, while producing confusion, inefficiency and demoralisation." -- Caius Petronius, Roman Consul, 66 A.D.
jhwurmbach wrote:
Out of dire need when the MS-C++ was so fundamentally broken as being incapable of compiling any STL.
It was created because there was no STL at the time. STL wasn't standard until 1999, MFC's collection classes have been around since 1994.
jhwurmbach wrote:
There is no need to use the afxtempl.h-collections in new software!
There is nothing fundamentally wrong with MFC's collection classes. They are perfectly fine, and unless you are actually making use of some of the more advanced features of STL, MFC's templates are even easier to use in some cases. You use the tool that fits you best for what you need to accomplish. To the OP, the biggest problem with the code is the member variables in the derived classes hiding the one in the base class. A better way to do this kind of thing is:
class A { public: A(int type) : _Type(type) {} virtual ~A() {} virtual int getType() = 0; }; class B : public A { public: B() {} virtual ~B() {} virtual int getType() { return TYPEB; } }; class C : public A { public: C() {} virtual ~C() {} virtual int getType() { return TYPEC; } };If you decide to become a software engineer, you are signing up to have a 1/2" piece of silicon tell you exactly how stupid you really are for 8 hours a day, 5 days a week Zac
-
bolivar123 wrote:
Use of CArray vs. using std::vector has nothing to do with the point I was trying to make.
Granted, but is is my habit to object against
CArraywhenever I see its use.:rolleyes: It was created with the early MFCs. Out of dire need when the MS-C++ was so fundamentally broken as being incapable of compiling any STL. There is no need to use the afxtempl.h-collections in new software! You are right withfor (int i=0;imyType)
{
case TYPEA:
DoSomethingWithTypeA(pA);
break;
case TYPEB:
DoSomethingWithTypeB((B*)ptr);
break;
case TYPEC:
DoSomethingWithTypeC((C*)ptr);
break;
}
}pA->myTypewill always beTYPEA(or undefined, with your unmodified code from the first posting). I had been misled by the debugger (VC2003), which is capable of showing the virtual variable (TYPEB, TYPEC) when the mouse is hovering overpA. When hovering overpA->myType, the correct value is shown, though. This whole issue is one of the confusing points of the C++ typesystem, but not a subtle bug, I think. You ordered a A*, so you got an A*.
"We trained hard, but it seemed that every time we were beginning to form up into teams we would be reorganised. I was to learn later in life that we tend to meet any new situation by reorganising: and a wonderful method it can be for creating the illusion of progress, while producing confusion, inefficiency and demoralisation." -- Caius Petronius, Roman Consul, 66 A.D.
jhwurmbach wrote:
I had been misled by the debugger (VC2003), which is capable of showing the virtual variable (TYPEB, TYPEC) when the mouse is hovering over pA.
What is a "virtual variable"? I don't think that there is any such beast. Mike
-
jhwurmbach wrote:
I had been misled by the debugger (VC2003), which is capable of showing the virtual variable (TYPEB, TYPEC) when the mouse is hovering over pA.
What is a "virtual variable"? I don't think that there is any such beast. Mike
Mike O`Neill wrote:
What is a "virtual variable"? I don't think that there is any such beast.
No - there isn't. Thats the point of this example. But the debugger shows - when the mouse hovers over
pA- the valueTYPEBorTYPECfor the variablemyType- which is undefined, as the experiment with theswitch/caseshows. .
"We trained hard, but it seemed that every time we were beginning to form up into teams we would be reorganised. I was to learn later in life that we tend to meet any new situation by reorganising: and a wonderful method it can be for creating the illusion of progress, while producing confusion, inefficiency and demoralisation." -- Caius Petronius, Roman Consul, 66 A.D.
-
Consider the following simple Win32 console app. It defines 3 classes A, B, and C. The classes B and C derive from class A. We create a CArray of class A pointers. Since B and C are derived from A, we can add pointers to B and C to the CArray. On the lines with "LINE A", "LINE B", and "LINE C" comments, what would the value of pA->myType be in each case? Hint, it won't be TYPEA, TYPEB, or TYPEC! To test this app, set breakpoints at each of the 3 lines with the calls to GetAt, and view the value of pA->myType in the debugger.
// FooTest.cpp : Defines the entry point for the console application.
//#include "stdafx.h"
#include "FooTest.h"
#include <afxtempl.h>#ifdef _DEBUG
#define new DEBUG_NEW
#endif// The one and only application object
CWinApp theApp;
using namespace std;
enum TClassType {TYPEA, TYPEB, TYPEC};
class A
{
public:
A() {};
~A();
TClassType myType;
};class B : public A
{
public:
B() { myType = TYPEB; }
~B();
TClassType myType;
};class C : public A
{
public:
C() { myType = TYPEC; }
~C();
TClassType myType;
};int _tmain(int argc, TCHAR* argv[], TCHAR* envp[])
{
int nRetCode = 0;
CArray<A*> myArray;
A* pA;myArray.Add(new A); myArray.Add(new B); myArray.Add(new C); pA = myArray.GetAt(0); //LINE A pA = myArray.GetAt(1); //LINE B pA = myArray.GetAt(2); //LINE C // initialize MFC and print and error on failure if (!AfxWinInit(::GetModuleHandle(NULL), NULL, ::GetCommandLine(), 0)) { // TODO: change error code to suit your needs \_tprintf(\_T("Fatal Error: MFC initialization failed\\n")); nRetCode = 1; } else { // TODO: code your application's behavior here. } return nRetCode;}
<Not meaning to be mean> <But...> Well, not sure this belongs to subtle bugs. The repetition of the same variable names would push it to bad design. And the unitialized member variable in the base class pushes it to bad code. </...But> <Not even> Talking about the lack of virtual destructors in a class hierarchy. Or <Ksss...> The use of public member variable. </...Ksss> </Not even> </Not meaning to be mean> <Feels like...> X| </...Feels like>
-
Oh goodie! If they don't plan on changing CArray, I'll continue using it. I just hate it when they go changing a good thing. Like the changes in CString from MFC 6 to MFC 7. Geez, I didn't think the original post would tangent to a debate on which is better......
bolivar123 wrote:
I didn't think the original post would tangent to a debate on which is better.....
Yes, it shouldn't really have done this as, for example, even if STL is the recommended way you could quite validly have been maintaining legacy code. Nonetheless, as an aside, I agree with the other guys that, other things being equal, we should prefer the STL - certainly for new projects. It's not as though this stuff has only just come out. All experienced C++ developers should be comfortable enough with it by now. OTOH, I'm not that religious about it. For example, although I haven't done any MFC for ages, I did still prefer CString to string.
Kevin
-
<Not meaning to be mean> <But...> Well, not sure this belongs to subtle bugs. The repetition of the same variable names would push it to bad design. And the unitialized member variable in the base class pushes it to bad code. </...But> <Not even> Talking about the lack of virtual destructors in a class hierarchy. Or <Ksss...> The use of public member variable. </...Ksss> </Not even> </Not meaning to be mean> <Feels like...> X| </...Feels like>
It is bad design and/or bad code for new developed code, but if you're cleaning up/encapsulating somebody else's code (who is no longer with the company)... BTW, does the incorrect use of XML tags make your post bad code?
-
It is bad design and/or bad code for new developed code, but if you're cleaning up/encapsulating somebody else's code (who is no longer with the company)... BTW, does the incorrect use of XML tags make your post bad code?
I expect those will soon be Microsoft extensions to XML.
-
I expect those will soon be Microsoft extensions to XML.
Until then, it'll be a not so subtle bug :laugh:
-
It is bad design and/or bad code for new developed code, but if you're cleaning up/encapsulating somebody else's code (who is no longer with the company)... BTW, does the incorrect use of XML tags make your post bad code?
bolivar123 wrote:
incorrect use of XML tags
Are you talking about the "..."? That was done on purpose... But I'm sure you'll gladly encapsulate it into a new post :)
-
I expect those will soon be Microsoft extensions to XML.
Actually this is not XML, this an extension to plain English. :)
-
Consider the following simple Win32 console app. It defines 3 classes A, B, and C. The classes B and C derive from class A. We create a CArray of class A pointers. Since B and C are derived from A, we can add pointers to B and C to the CArray. On the lines with "LINE A", "LINE B", and "LINE C" comments, what would the value of pA->myType be in each case? Hint, it won't be TYPEA, TYPEB, or TYPEC! To test this app, set breakpoints at each of the 3 lines with the calls to GetAt, and view the value of pA->myType in the debugger.
// FooTest.cpp : Defines the entry point for the console application.
//#include "stdafx.h"
#include "FooTest.h"
#include <afxtempl.h>#ifdef _DEBUG
#define new DEBUG_NEW
#endif// The one and only application object
CWinApp theApp;
using namespace std;
enum TClassType {TYPEA, TYPEB, TYPEC};
class A
{
public:
A() {};
~A();
TClassType myType;
};class B : public A
{
public:
B() { myType = TYPEB; }
~B();
TClassType myType;
};class C : public A
{
public:
C() { myType = TYPEC; }
~C();
TClassType myType;
};int _tmain(int argc, TCHAR* argv[], TCHAR* envp[])
{
int nRetCode = 0;
CArray<A*> myArray;
A* pA;myArray.Add(new A); myArray.Add(new B); myArray.Add(new C); pA = myArray.GetAt(0); //LINE A pA = myArray.GetAt(1); //LINE B pA = myArray.GetAt(2); //LINE C // initialize MFC and print and error on failure if (!AfxWinInit(::GetModuleHandle(NULL), NULL, ::GetCommandLine(), 0)) { // TODO: change error code to suit your needs \_tprintf(\_T("Fatal Error: MFC initialization failed\\n")); nRetCode = 1; } else { // TODO: code your application's behavior here. } return nRetCode;}
The value-of pA->myType is the value-of pA->A::myType which has not been initialized. Some changes: class A {public: A(){/*myType = TYPEA;*/} ~A(){} TClassType myType;}; class B : public A {public: B(){ myType = TYPEB; } ~B(){} TClassType myType;}; class C : public A {public: C(){ myType = TYPEC; } ~C(){} int myType;}; A* pA; B* pB; C* pC; pA = myArray.GetAt(0); //LINE A pA = myArray.GetAt(1); //LINE B pB = (B*)pA; TClassType eAB = pB->myType; TClassType eAA = pB->A::myType; pA = myArray.GetAt(2); //LINE C pC = (C*)pA; TClassType eAC = TClassType)pC->myType; eAA = pC->A::myType; Results: TYPEB == pB->myType // B::myType 2 == pC->myType // C::myType, 2==TYPEC ? == pA->myType // A::myType when uninitialized TYPEA == pA->myType // A::myType when initializion is uncommented Comments: Data-members are hidden or overridden. "Plain" function-members are hidden or overridden. Virtual function-members are virtual (behavior you expected). Hiding/overriding depends on NAME not type (see TClassType versus int). More comments: Such hiding is probably most often not done on purpose. Therefore Hiding data-members is 99.9999% (sure to result in) a bug. Hiding function-members is 99% a bug.