Nasty OO design issues

Started by
17 comments, last by Zahlman 19 years, 3 months ago
@Morbo, That is exactly what I needed. The change in nomenclanture (from "SomethingObject" to "Controller" reflects this pattern perfectly. I'll use this

@Oluseyi, SiCrane, Thanks, I understand how I screwed it up now. You guys have been a great help in figuring this mess out
Advertisement
@Morbo,
class Controller{protected:    TangibleObject* _object;public:    Controller(TangibleObject* object):_object(object) {}    virtual void pitch(double rad)=0;    virtual void yaw(double rad)=0;    virtual void roll(double rad)=0;    virtual void move(vector3 d)=0;    vector3 right();    vector3 up();    vector3 forward();};class NaturalController : public Controller{public:    void pitch(double rad);    void yaw(double rad);    void roll(double rad);    void move(vector3 d);};//preferred way, but this probably won't work cause virtuals only work with pointers, right?Controller control=&NaturalController(camera1);error C2440: 'type cast' : cannot convert from 'Camera *' to 'NaturalController'//second best wayController* c = new NaturalController(camera1);error C2664: 'NaturalController::NaturalController' : cannot convert parameter 1 from 'Camera *' to 'const NaturalController &'//only if i have to...Controller* c = new NaturalController((TangibleObject*)camera1);error C2664: 'NaturalController::NaturalController' : cannot convert parameter 1 from 'TangibleObject *' to 'const NaturalController &'

How do I resolve these

It's late and I'm tired and this will probably be obvious to me tomorrow morning but I cant for the life of me get this to compile tonight
TangibleObject* camera=new Camera; //create a new object
Controller *control=(NaturalController*)camera; // Pointer, not instance to the control ??
control->move(vector3(-.5,-.5,0)); //move it
Quote:Original post by thedustbustr
@Morbo,
*** Source Snippet Removed ***
How do I resolve these

It's late and I'm tired and this will probably be obvious to me tomorrow morning but I cant for the life of me get this to compile tonight


Constructors are not inherited. You have defined
Controller(TangibleObject* object)
but you are trying to call
NaturalController(TangibleObject* object)
which you never defined. Add this to the definition of NaturalController and the second initialization method you showed should work:
NaturalController(TangibleObject* object) : Controller(object) {}
Constructors are not inherited, but you can explicitly call the constructor of any parent class like I just showed. In fact, for all classes that do not have default constructors, you will be required to (assuming one of their constructors is not called by another parent constructor lower in the hierarchy but still in the class in question's ancestry).
As has been stated numerous times, your casting doesn't make sense. Your camera is simply NOT a NaturalController. A Camera in your design is a VisibleObject which is a TangibleObject which is an Object. NaturalController is no where in the direct Camera hierarchy, therefore a Camera is not a kind of NaturalController (nor does it support a NaturalController interface).

It seems to me like what you are doing is attempting to make a multiple-inheritance relationship, where a camera is both a NaturalController AND a VisibleObject, only you don't fully understand the flow and meaning of an inheritance hierarchy yet. What you probably were attempting to do was this:

class Object{public:    unsigned int id;    std::string name;    Object(std::string str="");    virtual ~Object();};//has an orientation and position in the worldclass TangibleObject : public Object{protected:         matrix44 _world;public:    matrix44 world(); //accessor for _world    void scale_local     (const double magnitude);    void rotate_local    (const vector3& axis, const double radians);    void translate_local (const vector3& v);    void scale_world     (const double magnitude);    void rotate_world    (const vector3& axis, const double radians);    void translate_world (const vector3& v);    vector3 right();    vector3 up();    vector3 forward();    vector3 global_right();    vector3 global_up();    vector3 global_forward();    TangibleObject();    void dump(int x, int y); //debugging (x,y)=screen pixels};class Controller : public virtual TangibleObject // So children which use multiple inheritance share one TangibleObject{public:    virtual void pitch(double rad)=0;    virtual void yaw(double rad)=0;    virtual void roll(double rad)=0;    virtual void move(vector3 d)=0;};class NaturalController : public Controller{public:    void pitch(double rad);    void yaw(double rad);    void roll(double rad);    void move(vector3 d);};//visibleclass VisibleObject : public virtual TangibleObject // So children which use multiple inheritance share one TangibleObject{ public:     virtual void draw()=0;    VisibleObject();    ~VisibleObject(){}};class Camera : public VisibleObject, public NaturalController // Both a VisibleObject and a NaturalController{public:    void draw();};int main(){  Camera blah;  TangibleObject& camera = blah; //create a new object  Controller& control = dynamic_cast< NaturalController& >( camera ); //take control of the object  // Or, more simply:  Controller& your_control = blah;}


In that case, your casting makes sense, since a Camera is now both a VisibleObject AND a NaturalController. However, while this works, I would personally consider it a bad design as there are more likely much better ways to accomplish what you are attempting to do, as was mentioned in this thread.

Quote:Original post by Morbo
Casting 'sideways', as you put it, simply doesn't work. In your instance, a Camera is NOT a Controller (in other words, Controller is not in Cameras hierarchy, either up or down). You can ONLY cast up the hierarchy (which always works), or down (only if you KNOW for certain that the object is of that type).

Just noting that you can do sideways casting (called cross-casting) with a dynamic_cast, however, not in the manner he is attempting to do. You can cross-cast when a type directly has multiple bases using multiple inheritance and you need to cast from one of those bases to the other. Again, though, that is often a sign of bad design unless you are absolutely positive it's what you should be doing (i.e. a good use can be casting between interfaces of an object whose updated versions of their interface branches out over time).
Quote:a good use can be casting between interfaces of an object whose updated versions of their interface branches out over time
Would you mind explaining that a bit more fully?
This snippet crashes on the third line in Release mode only. Debug mode works fine.
    Camera* camera1=new Camera;    Controller* control=&NaturalController(camera1);    control->move(-control->forward()*20);Unhandled exception at 0x004056a0 in katana4.exe: 0xC0000005: Access violation reading location 0x0000000c.


Why doesn't this work in Release?
thedustbustr: Sorry for not mentioning needing to have NaturalController define a constructor as well, and thanks to mumpo for stepping for that one.

The problem is with the second line:

Controller* control=&NaturalController(camera1);

It should be:

Controller* control=new NaturalController(camera1);

Or you could do:

NatrualController control(camera1);

If you don't need the controller to be dynamic.
As you had it, you were grabbing the address of a temporary object -- the NaturalController(camera1) -- which becomes invalid almost immediately. Debug mode tends to allow this stuff because of various memory padding it uses.



Have you considered that the problem might better be tackled by delegation (object composition) rather than inheritance?

This topic is closed to new replies.

Advertisement