5

This is NOT C++11

I'm interested in the 3rd parameter of Microsoft's CMapStringToOb::GetNextAssoc, which has following definition:

 void GetNextAssoc(
   POSITION& rNextPosition,
   CString& rKey,
   CObject*& rValue 
) const;

Then I've got following easy code for testing: two good cases and one case with compiler error.

class CMyObject : public CObject    //in order to use CMapStringToOb
{
public:
    CMyObject(CString name_)
        :name(name_)
    {
    }

    void SayHello()
    {
        TRACE(_T("hello") + name);
    }
    CString name;   
};

void main()
{
    CMapStringToOb myMap;
    myMap.SetAt(_T("a"), new CMyObject(_T("aaa")));
    myMap.SetAt(_T("b"), new CMyObject(_T("bbb")));
    myMap.SetAt(_T("c"), new CMyObject(_T("ccc")));

    //good case 1
    POSITION pos = myMap.GetStartPosition();
    while (pos)
    {
        CString s;
        CMyObject* pMine = NULL;
        myMap.GetNextAssoc(pos, s, (CObject*&)pMine);
        if(pMine)
        {
            pMine->SayHello();
        }
    }

    //good case 2
    pos = myMap.GetStartPosition();
    while (pos)
    {
        CString s;
        CObject* pObject = NULL;
        myMap.GetNextAssoc(pos, s, pObject);
        if(pObject)
        {
            CMyObject* pMine = static_cast<CMyObject*>(pObject);
            pMine->SayHello();
        }
    }

    //bad case:
    //can not compile
    //    error C2440: 'static_cast' : cannot convert from 'CMyObject *' to 'CObject *&'    
    //        static_cast and safe_cast to reference can only be used for valid initializations or for lvalue casts between related classes 

    pos = myMap.GetStartPosition();
    while (pos)
    {
        CString s;
        CMyObject* pMine = NULL;
        myMap.GetNextAssoc(pos, s, static_cast<CObject*&>(pMine));  //compile error
        if(pMine)
        {
            pMine->SayHello();
        }
    }   
}

All I was trying to do is find an proper way to replace the C style casting to C++ style cast in this case.

Reading from this, it mentioned:

C casts are casts using (type)object or type(object). A C-style cast is defined as the first of the following which succeeds:

const_cast
static_cast (though ignoring access restrictions)
static_cast (see above), then const_cast
reinterpret_cast
reinterpret_cast, then const_cast

Q1: Was the above list missing anything (e.g. for rValue)?

Q2: What's the proper way of translate C style cast to C++ style cast in this case ? (good case 2 works, but, is there a more concise one?)

Q3: How is the C Style cast doing for rValue? (in other words, please explain why good case 1 works)

Community
  • 1
  • 1
milesma
  • 308
  • 3
  • 11

3 Answers3

3

You can't static_cast between references (or pointers) to "unrelated types." While you could static_cast from a CMyObject* to a CObject*, that isn't what you're doing here. Here you're trying to cast a reference to a pointer into a reference to another pointer. And the two pointer types do not have an inheritance relationship.

I like your "good case 2" code--I'd run with that.

For more details on the non-relatedness of your pointer types, see here: static_cast and reference to pointers

Community
  • 1
  • 1
John Zwinck
  • 239,568
  • 38
  • 324
  • 436
  • Great! I was fooled by the *& signature, I re-looked this one by **, then it is getting quite obvious. BTW, I've also post an answer to this question to explain this. Thanks! – milesma Apr 29 '16 at 05:44
  • It's usually a good idea to assert, that a pointer points to a particular type prior to performing a downcast. MFC's `CObject` implements the [CObject::IsKindOf](https://msdn.microsoft.com/en-us/library/b7tsah76.aspx) function. Adding an `ASSERT(pObject->IsKindOf(RUNTIME_CLASS(CMyObject)));` before the `static_cast` is one way to achieve this. – IInspectable May 01 '16 at 16:32
1

The proper way to write that code would be to use std::map<>. Even if you insist on keeping the existing code mostly, consider fixing the interface of GetNextAssoc() to just return the pointer. In order to do that, you could simply add an overload of that function:

CObject* GetNextAssoc(
    POSITION& rNextPosition,
    CString& rKey,
) const {
    CObject* res = 0;
    GetNextAssoc(rNextPosition, rKey, res);
    return res;
}

Even more, you could template that function and do the conversion to the target type there. Also, you could then use dynamic_cast, which should be used because formally, the container stores CObjects and they could have various, different types.

Now, why did I partially ignore your question? The reason is that the MFC don't follow modern coding style and in some cases, they simply do things that are frowned on. There are a bunch of justifications for that behaviour, but foremost it is age (didn't know better, didn't have proper template support) combined with compatibility concerns (can't change that now). That's not a reason to repeat these mistakes though.

Ulrich Eckhardt
  • 16,572
  • 3
  • 28
  • 55
1

Inspired by John Zwinck, I will look from a different angle:

static_cast<CObject*>(pMine) 

will succeed because type "CMyObject" generalize from type "CObject"; actually, this is done implicitly;

static_cast<CMyObject*>(pObject) 

will succeed because type "CMyObject" generalize from type "CObject";

static_cast<CObject**>(&pMine) 

will FAIL because type "CMyObject*" does NOT generalize from type "CObject*";

reinterpret_cast<CObject**>(&pMine) 

will succeed at compile time because of "reinterpret_cast"; how about run time?

Let's make an assumption of the possible new implementation:

void CMapStringToOb::GetNextAssoc(
        POSITION& rNextPosition,
        CString& rKey,
        CObject** ppValue)
{
    *ppValue = (the pointer at the current position, point to an instance of "CMyObject");
}

So with calling this function by:

GetNextAssoc(pos, s, reinterpret_cast<CObject**>(&pMine))

the result is that "pMine" is pointing to an instance of "CMyObject";

So runtime is SAFE.

However, if we insert the key-value by (Note: CYourObject has no generalize relationship to CMyObject)

myMap.SetAt(_T("a"), new CYourObject(_T("aaa")));

and get it out by

GetNextAssoc(pos, s, reinterpret_cast<CObject**>(&pMine));

Compile time will still succeed, however, pMine is now pointing to "CYourObject", which will be UNDEFINED BEHAVIOR at runtime. (static_cast has the same issue, though)

milesma
  • 308
  • 3
  • 11