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 MSG:
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.

Reference:
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?

Thanks in advance.

Answer 1

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.

Answer 2

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);
}

Answer 3

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.

Answer 4

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.