Sign in to follow this  
Lode

virtual functions

Recommended Posts

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

Share this post


Link to post
Share on other sites
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++.

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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 with
typedef virtual_wrapper<Test1> Test;

Test test(new Test2());
Test other(test); // check copy construction
std::vector<Test> omg;
omg.push_back(test);
omg.push_back(other);
omg[1]->x();
// Everything cleaned up nicely at end of scope.

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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

Share this post


Link to post
Share on other sites
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

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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 ;)

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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)...

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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];
};

Share this post


Link to post
Share on other sites
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";
}
};

Share this post


Link to post
Share on other sites
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 :)

Share this post


Link to post
Share on other sites
Quote:
Original post by Grammarye
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.

I would disagree that it's odd to call a virtual function from one that overrides it. You might want to extend the virtual function's behaviour by simply adding to it, while keeping the super- and sub-class versions of the function de-coupled.

Share this post


Link to post
Share on other sites
Quote:
However, I assume VC8, just like GCC, will throw up errors if you try to preform any interesting operation on it


Not really. Some operations fail because auto_ptr lacks a default constructor, but it does have a copy constructor and assignment operator. resize won't work, but push_back would.

Share this post


Link to post
Share on other sites
Quote:
Original post by Deniz
Quote:
Original post by Grammarye
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.

I would disagree that it's odd to call a virtual function from one that overrides it. You might want to extend the virtual function's behaviour by simply adding to it, while keeping the super- and sub-class versions of the function de-coupled.


I have to agree with this statement. Have you ever extended System::Windows::Forms::Control::OnResize (Replace Control with any windows control and OnResize with any message that you might recieve from that control)? You are practically required to call __super::OnResize to ensure that the delagate is called. In fact, this is recommended in the help files for .Net.

I have had many occasions in native C++ where I derived off an object that derived off my BaseObject and extended some of the virtual functions rather than replacing them.

The only time refactoring is necessary is when you only want to use portions of the parent code, and you have access to the parent code (Which you don't in my example). If it's okay to use the entirety of the parent code, then why incur the readability hit and compiler/optimization time of an extra, unneccessary function? However, if you need to use only a portion of the parent code, by all means, refactor.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this