views:

584

answers:

6

Here's my problem: I have a virtual method defined in a .h file that I want to call in a class that inherits from the base class. Sadly though, the method in the derived class doesn't get called. Is there a better way to implement what I'm trying to do?

    #ifndef ofxBASE_SND_OBJ
    #define ofxBASE_SND_OBJ

    #include "ofConstants.h"

    class ofxBaseSndObj {

    public:

        virtual string getType(){}

        string key;

    };

#endif

Here's my buzz class

#ifndef OFXSO_BUZZ
#define OFXSO_BUZZ

#include "ofxBaseSndObj.h"

class ofxSOBuzz : public ofxBaseSndObj
{
public:
    string getType();
};

#endif

ofxSOBuzz.cpp

string ofxSOBuzz::getType()
{
    string s = string("ofxSOBuzz");
    printf(" ********* returning string type %s", s.c_str()); // doesn't get called!
    return s;
}

Then in another class I try to call it this way:

string ofxSndObj::createFilter(ofxBaseSndObj obj)
{
    string str = obj.getType();
    if(str.compare("ofxSOBuzz") == 0)
    {
     printf(" all is well ");
    }
}

In the method above I need to be able to pass in one of many kinds of objects that all extend the ofxBaseSndObj object. Any suggestsions or pointers would be greatly appreciated. Thanks!

+10  A: 

You need to pass the instance to createFilter as a pointer (or reference) to the object. You are passing by value, and this causes the compiler to copy the derived object you use as an argument into an instance of the base class. When it does this you lose the fact that it was originally a derived type.

As written your code shouldn't actually compile since the declaration of ofxBaseSndObj::getType doesn't return anything. Did you mean for this to be an abstract method or return an empty string?

If you made it an abstract method then the compiler would complain about trying to instantiate an abstract class in your ofxSndObj::createFilter method.

Rob Walker
Yeah you're right, wierd. It shouldn't compile...odd. I'm on gcc4.2, I wonder if someone else has run into this?
Joshua Noble
+23  A: 

Change this line:

string ofxSndObj::createFilter(ofxBaseSndObj obj)

to

string ofxSndObj::createFilter(ofxBaseSndObj& obj)

What you are doing is passing by value (passing a copy).

This means you are copying the object to the function. Because the function does not know what type you are actually passing it only passes the type defined in the function declaration and thus it makes a copy of the base class (this is know as the slicing problem).

The solution is to pass by reference.

If you do not want the function to modify the object (maybe that is why you were passing by value so it could not alter the original) then pass a const reference.

class ofxBaseSndObj
{
    public:
        virtual string getType()  const;
        // If the method does not change the object mark it const

        string key;

};

string ofxSndObj::createFilter(ofxBaseSndObj const& obj)
{
    // allowed to call this if getType() is a const
    string str = obj.getType();

    if(str.compare("ofxSOBuzz") == 0)
    {
        printf(" all is well ");
    }
}
Martin York
+2  A: 

This problem is called "slicing" in C++.

Greg Hewgill
+2  A: 

Making the copy constructor and operator= private is an effective way of preventing this bug from happening again.

For example:

class ofxBaseSndObj {
public:
    virtual string getType(){}
    string key;

private:
    ofxBaseSndObj(const ofxBaseSndObj& rhs);
    ofxBaseSndObj& operator=(const ofxBaseSndObj& rhs);
};

If there is no other good reason you should use C++'s built in RTTI. You can then use typeid operator. Look at your compilers documentation to turn this on if it is not on by default.

Dave Hillier
Yeah, I'm going to redesign to just use pointers and RTTI instead. I wanted to avoid passing by reference (long story, the API has an odd user base) but I think it's the only way it's going to work and it's the way that makes the most sense. Thanks!
Joshua Noble
A: 

You could use dynamic_cast or type_id

+1  A: 

Others have addressed the slicing problem. You then ask Ok, let me say, I know I need to do something to determine the base type, but is there something more elegant than doing an enum lookup to determine the kind of inherited object?

Querying and switching on the type of the object is a poor design which misses the point of the OO approach.

Instead of

string ofxSndObj::createFilter(ofxBaseSndObj& obj)
{
    string str = obj.getType();
    if(str.compare("ofxSOBuzz") == 0)
    {
        // do ofxSOBuzz - specific thing
    }
    else if(str.compare("some other derived class") == 0)
    {
        // do stuff for other derived classes
    }
       // etc...
}

make the interesting behaviour the virtual function:

class ofxBaseSndObj {

public:
    // get rid of getType()
    virtual void HelpCreateFilter() = 0;
};


string ofxSndObj::createFilter(ofxBaseSndObj& obj)
{
    // Let the derived class do it's own specialized work.
    // This function doesn't need to know what it is.
    obj.HelpCreateFilter();
    // rest of filter creation
}

Why is this better than the original version? Because ofxSndObj::createFilter does not need modifying if future derived classes of ofxBaseSndObj are added to the system. Your version needs extending for each new derived class. If this is unclear, try to post a little more code - I can't tell from your code or class names what these functions are supposed to do.

fizzer
He's gone away - and taken his question with him.
fizzer