views:

608

answers:

5

I have a question about the following code:

class MyClass : private boost::noncopyable
{
    public:

    MyClass() {}
    virtual ~MyClass() {}
}

class OtherClass : private boost::noncopyable
{
    private:
    MyClass* m_pMyClass;
}

My thoughts are that MyClass cannot be copied using construction or assignment. Using a virtual destructor is needed if I want to support deriving classes from MyClass, which I do not want to support. I do not intend to create pointers to this class and pass them around.

I do not want a Singleton and I cannot see a downside to removing the virtual destructor.

Do I introduce a potential problem if remove the virtual destructor for a noncopyable class? Are there a better practices to handle a class that does not need to be a Singleton, but I only want one instance in another class and not support inheritance?

+12  A: 

No, the entire point of a virtual destructor is so derived classes can properly destruct polymorphically. If this will never be a base class, you don't need it to be virtual.

Terry Mahaffey
+4  A: 

I'm really not a fan of the boost::noncopyable class as a whole. Why not just declare your class's copy constructor and assignment operator private and not define them. That will accomplish the same thing, and you can ditch the virtual destructor.

Boost only supplies a virtual destructor so that people can pass around boost::noncopyable objects polymorphic and still have them behave well. Technically speaking if you are not going to use the class polymorphically (you can even still inherit from it), you really don't need a virtual destructor.

Chris
Because boost::noncopyable is more readable, as they say themselfs.
Viktor Sehr
boost::noncopyable does not provide a virtual destructor
villintehaspam
Readability is in the eye of the beholder
anon
@villintehaspam: Seems to me it would be extremely unusual to want to delete using a pointer to boost::noncopyable.
Fred Larson
`MyClass` can't be deleted (by external agents) as a `boost::noncopyable` anyway since it uses private inheritance.
jamesdlin
@Fred Larson: Exactly, it doesn't really make sense - so there is no reason to provide it.
villintehaspam
+5  A: 

The general rule is if your class has virtual functions it needs a virtual destructor. If it doesn't, but is still derived from a base class, the base class (and hence your class) may or may not need a virtual destructor depending.

Deriving your class from boost::noncopyuable doesn't really count as being derived from a base class. boost::noncopyable is more like a convenient annotation backed up with a couple of declarations that will cause the compiler to enforce the annotation. It's not really a base class in any conventional sense. Nobody is ever going to try to pass around a pointer to your class as a pointer or reference to boost::noncopyable. And even if they did your virtual destructor wouldn't help because the boost::noncopyable's destructor isn't.

And lastly, as was pointed out in a comment, you are even privately inheriting from boost::noncopyable so it's not even really inheritance at all as far as anybody outside the class is concerned.

So really, no need to make it a virtual destructor.

Omnifarious
It is using private inheritance, from outside of the class, it will appear as not inheriting from `boost::noncopyable`
David Rodríguez - dribeas
+2  A: 

Virtual destructor in base class is used to avoid partial destruction problem like :

Base *pBase = new Derived();
delete pBase; 

//if destructor is not made virtual then derived class destructor will never called.

When you inherit a class privately , compiler won't perform implicit casting from derived to base class and if you are sure that derived object is never destructed using base class pointer then you don't need virtual destructor in base class.

Base *pBase = new Derived(); // will flash error 
Ashish
A: 

boost::noncopyable is meant to say that you don't want copies of the object made. You're aware that this is different from deriving from the object.

It is perfectly fine to get rid of the virtual destructor if you won't ever derive from the object. If you want to enforce the "don't derive from this object" policy, there is a way. Unfortunately, there is no boost::nonderivable to pretty this up for you (probably because it would be impossible to derive from a boost:nonderivable).

Max Lybbert