About downcasting from base class to subclass pointer

Question

A static check tool shows a violation on the below code:

class CSplitFrame : public CFrameWnd  
...
class CVsApp : public CWinApp
CWnd* CVsApp::GetSheetView(LPCSTR WindowText)
{
 CWnd* pWnd = reinterpret_cast<CSplitFrame*>(m_pMainWnd)->m_OutputBar.GetChildWnd(WindowText);
 return pWnd;
}

Error Message: Class 'CSplitFrame' inherits from class 'CWnd'

Description: Avoid casts down the inheritance hierarchy. This rule detects casts from a base class pointer to a subclass pointer.

Benefits: Allowing casts down the inheritance hierarchy leads to maintenance problems, and downcasting from a base class is always illegal.

References:

1. Scott Meyers, "Effective C++: 50 Specific Ways to Improve Your Programs and Design", Second Edition, Addison-Wesley, (C) 2005 Pearson Education, Inc., Chapter: "Inheritance and Object-Oriented Design", Item 39
2. JOINT STRIKE FIGHTER, AIR VEHICLE, C++ CODING STANDARDS Chapter 4.23 Type Conversions, AV Rule 178

Do you think it's a good practice for not casting down from a base class pointer to a subclass pointer? Why and When should I follow this rule?

Chatelain

Running a static analyzer on MFC code? You're joking, right? MFC itself breaks many JSF coding standards, the wizard-generated code breaks more, it's really not designed for type safety. – Essayist

BTW: it would be a good idea to stop calling derived classes subclasses. Derivation/inheritance is a syntactic concept, subclassing is a typing concept, and the two are more or less independent ideas. – Oenomel

Answer 1

Let us go through some of the downcasting example in MFC:

CButton* from CWnd*

CWnd* wnd = GetDlgItem(IDC_BUTTON_ID);
CButton* btn = dynamic_cast<CButton*>(wnd);

CChildWnd* from CFrameWnd*

CChildWnd * pChild = ((CSplitFrame*)(AfxGetApp()->m_pMainWnd))->GetActive();

There are indeed some of the limitation of MFC design.

Due to CWnd provides the base functionality of all window classes in MFC, it does even serve as a base class of View, Dialog, Button etc.

If we want to avoid downcasting, probably we need MFC hacker to split CWnd into fewer pieces?

Now, comes to the another question, how to solve the violation, my humble opinion is try to avoid unsafe downcasting, by using safe downcasting:

Parent *pParent = new Parent;
Parent *pChild = new Child;

Child *p1 = static_cast<Child*>(pParent);   // Unsafe downcasting:it assigns the address of a base-class object (Parent) to a derived class (Child) pointer
Parent *p2 = static_cast<Child*>(pChild);   // Safe downcasting:it assigns the address of a derived-class object to a base-class pointer

It serve as good practise for using safe downcasting, even though the violation is still exists, we will just suppress the violation with given explanation.

Few of the useful reference:
http://support.microsoft.com/kb/108587
http://blog.csdn.net/ecai/archive/2004/06/26/27458.aspx
http://www.codeproject.com/KB/mcpp/castingbasics.aspx
http://www.bogotobogo.com/cplusplus/upcasting_downcasting.html

Lastly, thanks for various useful response from all of you.
They are indeed very helpful.

Chatelain

Answer 2

reinterpret_cast is certainly a bad idea here, regardless of coding standards or OOP theory. It has to be dynamic_cast or boost::polymorphic_downcast.

As for the chapter 39 of Effective C++, it concentrates on the maintenance problems caused by having to downcast to multiple different types and having to check the return values of dynamic_cast for potential failures, resulting in multiple branches in the code:

The important thing is this: the if-then-else style of programming that downcasting invariably leads to is vastly inferior to the use of virtual functions, and you should reserve it for situations in which you truly have no alternative.

Benita

Or static_cast, if the dynamic type of the object is known. – Introduce

Actually, what I said is misleading. static_cast<T*>(p) can be used safely if *p is known to be a T. For that to be the case, T doesn't necessarily have to be the dynamic type of *p. T might also be a base class of the dynamic type of *p and a derived class of the static type of *p. T doesn't necessarily have to be the dynamic type of *p. – Introduce

Answer 3

It doesn't look to me like it is actually necessary for you to perform the cast anyway, or at least not as you are doing it. Your function needs to return a CWnd, so you don't need to cast up to a CSplitFrame.

I would have expected that GetChildWnd returns a CWnd* or similar; why can you not write something like:

CWnd* CVsApp::GetSheetView(LPCSTR WindowText)
{
   return m_pMainWnd->m_OutputBar.GetChildWnd(WindowText);
}

Pentameter

m_pMainWnd is a member of CWnd, which does not has access to m_OutputBar, which is a member of CSplitFrame. – Chatelain

Answer 4

Downcasting class pointers is generally something that should be avoided at all costs in properly designed OOP code. Nevertheless, sometimes such casts are necessary, especially if you are using some other library/code that was designed to require such casts in the client code. MFC is one example of such library.

When downcasts are really necessary, they should never be performed with reinterpret_cast. The proper way to perform the cast is either dynamic_cast or static_cast.

Quite often people use reinterpret_cast when they see a C-style cast used for downcast in the code and decide to convert it to C++-style cast, incorrectly assuming that a C-style cast in such context is equivalent to reinterpret_cast. In reality, C-style cast applied to parent-child type pair in any direction is equivalent to static_cast with some extra perks (C-style cast can break through access protection). So, again, using C-style casts for this purpose is wrong, but the proper C++ replacement is not reinterpret_cast, but rather dynamic_cast or static_cast.

Poland