0

I'm trying to create a function which is overloaded based on the specialization of its parameter, such as this:

class DrawableObject...;
class Mobile : public DrawableObject...;

class Game
{
    AddObject(DrawableObject * object)
    {
        // do something with object
    }
    AddObject(Mobile * object)
    {
        AddObject(dynamic_cast<DrawableObject *>(object));
        DoSomethingSpecificForSpecializedClass();
    }
};

...but my MS compiler is giving me this error:

error C2681: 'Mobile *' : invalid expression type for dynamic_cast

Both classes have virtual functions. Is this the wrong cast for up-casting in this situation? I have tried with a C-style cast and everything functions as intended. Also, are there any potential pit-falls with this design?

  • 1
    There is a spelling error in: `class Mobile : public DrawableObbject...;`. I am not sure if this is relevant but `DrawableObbject` should be `DrawableObject`. – Khaled Alshaya Jan 07 '10 at 21:47
  • Is AddObject(Mobile *) doing anything besides calling AddObject(DrawableObject *)? If not, I don't see the purpose of AddObject(Mobile *) – Brian Young Jan 07 '10 at 21:49
  • Fixed the typo, and elaborated for Brian Young's question. –  Jan 07 '10 at 21:53

5 Answers5

4

For explicit upcasting, use static_cast.

Your design should work fine. Be aware that calls to AddObject() will be ambiguous if you try to pass an object that can be implicitly converted to both Mobile* and DrawableObject*, such as a pointer to a class derived from Mobile.

Drew Dormann
  • 59,987
  • 13
  • 123
  • 180
  • Thank you, people who said to use static_cast. I guess it makes sense as it's guranteed to be of the correct type, that all will be well. Cheers! –  Jan 07 '10 at 22:02
1

As Neil stated, the cast is simply wrong. dynamic_cast<> is for downcasting from base towards derived not the other way around. A better approach is to factor out the common code like:

class Game {
protected:
    void commonAddObject(DrawableObject *obj) {
        // do common stuff here
    }
public:
    void addObject(DrawableObject *obj) {
        commonAddObject(obj);
        // do DrawableObject specific stuff here
    }
    void addObject(MobileObject *obj) {
        commonAddObject(obj);
        // do MobileObject specific stuff here
    }
};

or to create separate methods for DrawableObject and MobileObject that do not rely on overloading by types. I prefer to steer clear of casting altogether if I can.

Community
  • 1
  • 1
D.Shawley
  • 58,213
  • 10
  • 98
  • 113
0

The cast is wrong, and completely unecessary - Mobile already is a DrawableObject.

  • 4
    Without the cast it'll end up in infinite recursion and..bubah, StackOverflow, surely? –  Jan 07 '10 at 21:51
  • Maybe. I don't claim to understand your code - it seems completely wrong to me - have you heard of virtual functions? –  Jan 07 '10 at 21:58
  • Yes, and as stated in the text, the classes are using them. What seems wrong with having specific functionality based on how specialized a parameter is? –  Jan 07 '10 at 22:00
  • If you really want to do this, have a single function and call the specialised code conditionally on the success of dynamic casting to a more specialised type. But VF's are in general a better solution. –  Jan 07 '10 at 22:13
0

...or remove your AddObject(Mobile * object) overload entirely. Without that function there, it will be "implicitly casted" to it's base class, and the AddObject(DrawableObject*) function would have been called. There is no need for you to manually add an overload and a cast for each type in your hierarchy.

Edit Code was added, I want to clarify some suggestions about your design.

Either your "Game" class treats all objects uniformly, or it does not. If it does not, there is no point in providing a generic "AddObject" overload that is publicly available - you're already coupled tightly to the individual objects, so you might as well drop it and the charade of a loosely coupled design.. You could still have it as a private helper function, AddObjectInternal. Since it's not an overload, you won't need the cast to disambiguate the call.

If you are or hope to treat all objects uniformly, consider putting such logic that you're currently putting in the AddObject overloads into virtual functions on the object class. Then, you only have one AddObject method, which calls the virtual functions on the object added.

Terry Mahaffey
  • 11,775
  • 1
  • 35
  • 44
  • Ok, maybe my question wasn't clear enough. The idea was for the specialized class it'd do something on top of the AddObject(DrawableObject*)'s functioanlity - fixed! –  Jan 07 '10 at 21:52
  • Ha... I thought the same thing, Terry, and had only guessed that more code would be added. – Drew Dormann Jan 07 '10 at 21:56
  • Ah, I see. Ok. I think that's bad design though - I would suggest putting such logic in a virtual function on the DrawableObject class, rather than put that logic in your Game class. DrawableObject could have a default empty implementation, and subclasses can do whatever. Then Game::AddObject would call object->DoSomethingSpecific(); – Terry Mahaffey Jan 07 '10 at 23:12
0

Upcasting is always free and safe. That means you don't need to use a protected dynamic_cast. A static_cast would be the proper way to do this, although a c-style cast will work as well. In reality you should be able to do away with the second AddObject function because if you passed a pointer to a Mobile object into the DrawableObject function it would call the proper function without needing any casting. Unless you are planning on putting specialized functionality in the overloaded function I wouldn't write it.

Beanz
  • 1,957
  • 1
  • 13
  • 14