🔒 Locked

virtual functions

Started by Lode Nov 10, 2005 at 10:43 AM 22 replies 3.2k views

Original Post

Nov 10, 2005 at 10:43 AM

In the following code


class Test1
{
    public:
    virtual void x()
    {
        std::cout << "1";
    }
};

class Test2 : public Test1
{
    public:
    void x()
    {
        std::cout << "2";
    }
};

int main(int argc, char *argv[])
{    

    Test2 test;
    std::vector<Test1> omg;
    std::vector<Test1*> omgp;
    omg.push_back(test);
    omgp.push_back(&test);
    test.x(); //this prints 2
    omg[0].x(); //this prints 1 while I'd like to to print 2
    omgp[0]->x(); //this prints 2
}

I'd like the "omg[0].x();" thing to print 2 as well. Is there a way to do that? (without pointers). Thanks

jpetrie

Nov 10, 2005 at 10:52 AM

The virtual dispatch mechanism happens only through pointers.

In your example omg is a vector of Test1 objects... these objects are actual, concrete Test1's (as opposed to omgp which holds pointers to Test1s, so the actual object pointed to by any given Test1 pointer may actually be a Test2).

Since you don't provide a copy ct in your classes, the compiler is generating one for you, and when you are pushing-back your Test2 into a vector of Test1's you are most likely slicing your Test2 (this is a bad thing).

You can invoke Test1::x() from Test2::x() if you like, but I don't think that will be accomplishing what you want. It sounds like you want polymorphic behavior through non-pointers... which you can't really get in C++.

ZQJ

Nov 10, 2005 at 10:53 AM

No, there isn't (maybe boost::any but that's a different thing). omg is a vector of Test1 objects so when you insert the Test2 object into it a Test1 is copy-constructed inside the vector. That means that what you have in the vector simply is a Test1 not a Test2, so you'll always get 1.

Zahlman

Nov 10, 2005 at 11:27 AM

Welcome to the hell of object slicing. This is why garbage collection is so nice in the languages what use it (you can just store pointers to anything, and often implicitly do without realizing it).

One option is to wrap a pointer in your own class, to provide RAII and make the pointer 'ownership' more explicit - that way you have object rather than pointer behaviour, but still access to the polymorphism (although you still have to write -> rather than .). However, doing it properly and allowing for copying will require the 'virtual clone idiom'; I don't know a particularly nice way to do it, but here's something off the top of my head:

template <typename base>class virtual_wrapper {  base* wrapped;    public:  // Not sure about these  base& operator*() { return wrapped; }  const base& operator*() const { return wrapped; }  // There should be an operator-> too but I don't understand how those work  // Warning! Ctor will take ownership of the raw pointer.  virtual_wrapper(base* thing) : wrapped(thing) {}  virtual_wrapper(const virtual_wrapper& other) : wrapped(other->clone()) {}  virtual_wrapper& operator=(const virtual_wrapper& rhs) {    wrapped = other->clone();  }}class Test1{    public:    // etc.    // need to write this for each class :(      virtual Test1* clone() { return new Test1(this); }};class Test2 : public Test1{    public:    // etc.    // need to write this for each class :(      virtual Test2* clone() { return new Test2(this); }};// Now we can work withtypedef virtual_wrapper<Test1> Test;Test test(new Test2());Test other(test); // check copy constructionstd::vector<Test> omg;omg.push_back(test);omg.push_back(other);omg[1]->x();// Everything cleaned up nicely at end of scope.

Promit

Nov 10, 2005 at 11:38 AM

The previous poster mentioned it, and I'd like to emphasize it.
You must avoid assigning a derived class to a base class by value.
When this happens, it's called slicing, because any data members of the derived class with be sliced off, and only the base class will be copied. Virtuality functions only through pointers and references. Be extremely careful when working with base class values; don't copy them as the base class, don't pass them by value to functions, don't insert them by value into containers, etc. Your omg vector is dangerous, and inserting a Test2 into it is a mistake. All of the information about Test2 is destroyed when you do that.

Oh, and if you want to attempt Zahlman's solution, be very careful. STL containers take some parameters by value instead of by const reference, so if your pointer wrapper's destructor deletes the pointer, you will always destroy your object when you attempt to add it to a container.

TfpSly

Nov 10, 2005 at 2:46 PM

class Test2 : public Test1{[...]    void x()[...]};// should be :class Test2 : public Test1{[...]    virtual void x()[...]};

else calling method x on an instance of test1 will always call Test1::x, as the VMT of the instance will NEVER contain a pointer on Test2::x, even for an instance of Test2.
So your test VMT is not initialised correctly for what you want to do.

std::vector should be able to store Test1/Test2 instances

Enigma

Nov 10, 2005 at 2:58 PM

Quote:
Original post by TfpSly
*** Source Snippet Removed ***
else calling method x on an instance of test1 will always call Test1::x, as the VMT of the instance will NEVER contain a pointer on Test2::x, even for an instance of Test2.
So your test VMT is not initialised correctly for what you want to do.

This is incorrect. Although it is generally considered bad form to omit the virtual specifier on such derived functions it such a specifier is not required. "Once virtual, always virtual", as the saying goes.

Enigma

RDragon1

Nov 10, 2005 at 3:02 PM

TfpSly: that virtual there is implicit. It's the same thing.

TfpSly: "std::vector should be able to store Test1/Test2 instances" - it "can", but it will almost never do what you want it to/expect it to

jpetrie: "The virtual dispatch mechanism happens only through pointers." ...and references

You guys need to be given a nice boost::shared_ptr

{   std::vector< boost::shared_ptr<Test1> > vec;   vec.push_back( boost::shared_ptr<Test1>( new Test1 ) );   vec.push_back( boost::shared_ptr<Test1>( new Test2 ) );   vec[0]->x(); //works correctly   vec[1]->x(); //works correctly   //everything gets cleaned up, too, no need to iterate through and delete each pointer}

CLEANER:

{   typedef boost::shared_ptr<Test1> pTest;   std::vector<pTest> vec;   vec.push_back( pTest( new Test1 ) );   vec.push_back( pTest( new Test2 ) );   vec[0]->x(); //works correctly   vec[1]->x(); //works correctly   //everything gets cleaned up, too, no need to iterate through and delete each pointer}

When you learn about shared_ptr, learn about weak_ptr, too. And don't forget std::auto_ptr, but don't try to put auto_ptr's into a container. Scott Meyers explains this stuff pretty well in his books.

demonkoryu

Nov 10, 2005 at 3:09 PM

Quote:
Original post by RDragon1
   typedef boost::shared_ptr<Test1> pTest;

Slightly OT, nitpicky and not even important,

Test_ptr

is IMO a better name than

pTest

, because the latter communicates a pointer variable (to me, because I've seen too much pseudo-hungarian notation), while the former looks more like a type.

RDragon1

Nov 10, 2005 at 3:44 PM

I very, very rarely declare a variable as a raw pointer, so for most intents and purposes a shared_ptr is as close as I usually see to a pointer. I use pType and wpType for shared_ptr and weak_ptr respectively. It's just what I like ;)

jpetrie

Nov 10, 2005 at 3:49 PM

Quote:

"The virtual dispatch mechanism happens only through pointers." ...and references

Very true, I should have pointed that out.

RDragon1

Nov 10, 2005 at 3:52 PM

That's okay, I did it for you ;)

Zahlman

Nov 10, 2005 at 6:34 PM

Quote:
Original post by Promit
The previous poster mentioned it, and I'd like to emphasize it.
You must avoid assigning a derived class to a base class by value.
When this happens, it's called slicing, because any data members of the derived class with be sliced off, and only the base class will be copied. Virtuality functions only through pointers and references. Be extremely careful when working with base class values; don't copy them as the base class, don't pass them by value to functions, don't insert them by value into containers, etc. Your omg vector is dangerous, and inserting a Test2 into it is a mistake. All of the information about Test2 is destroyed when you do that.

Oh, and if you want to attempt Zahlman's solution, be very careful. STL containers take some parameters by value instead of by const reference, so if your pointer wrapper's destructor deletes the pointer, you will always destroy your object when you attempt to add it to a container.

Yes; that's why I invoke clone() in the wrapper copy ctor, and expect the clone() to return a new copy-constructed instance of the object. The object's cctor semantics are supposed to be such that this is ok.

RDragon1

Nov 10, 2005 at 6:41 PM

Zahlman while I admire your effort, it would be hard to catch me using a smart pointer implementation that's not maintained by something like the boost project - their effort is a result of several years of experts and end users providing feedback, bug reports, and corrections to the implementation so that it's as bug free and as usable as possible - many realize that it's a very difficult task to implement one that works correctly in the grand scheme of things, and I wouldn't recommend rolling your own for the same reason you shouldn't roll your own version of many other things found in the standard libraries or in libraries such as the boost libraries... Leave the library writing up to library writers (or work on your skills to become one yourself)...

Promit

Nov 10, 2005 at 6:59 PM

Just don't anybody try to put a std::auto_ptr into an STL container. Wrong idea.

RDragon1

Nov 10, 2005 at 7:08 PM

I've yet to see if VC8 will allow it to compile - it's forbidden by the standard's rules, and hopefully most compilers will disallow it eventually.

Deyja

Nov 10, 2005 at 9:56 PM

It doesn't, which is rather disapointing. It would be simple to prevent the declaration of a container of auto pointers using a partial specialization that contained something as simple as a zero-length array data member. Naive example -

template <typename T> class vector<auto_ptr<T> > { int cant_make_container_of_auto_ptrs[0]; };

erissian

Nov 11, 2005 at 12:04 AM

It seems like you want to have the child class perform the actions of the base class, plus other stuff? This works fine for me:

class Test2 : public Test1{    public:    void x()    {        Test1::x();        std::cout << "2";    }};

MaulingMonkey

Nov 11, 2005 at 12:11 AM

Quote:
Original post by Deyja
It doesn't, which is rather disapointing.

Same with GCC. However, I assume VC8, just like GCC, will throw up errors if you try to preform any interesting operation on it (push_back, resize... anything potentially changing .size() to be something other than zero...)

Grammarye

Nov 11, 2005 at 5:14 AM

Quote:
Original post by erissian
It seems like you want to have the child class perform the actions of the base class, plus other stuff? This works fine for me:
class Test2 : public Test1{    public:    void x()    {        Test1::x();        std::cout << "2";    }};

Setting aside the slicing issues mentioned by others, that code wouldn't work as Lode appears to be desiring - you'd get 1 and 2 printed out on calling x(), not just 2. Plus it's a little odd to be calling a virtual function inside a function that is in theory overriding it. I'd argue that if you're needing code from a function that you're overriding, then it needs refactoring into a separate function that you're not overriding. It's also a lot clearer in maintenance.

Either way, he needs to be using something that is a pointer/reference to get virtual behaviour, whether a pointer directly in the vector or a suitable object that wraps a pointer that can be used in a vector (not auto_ptr in case anyone's missed the numerous references :)

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.