Sign in to follow this  

Herb Sutter: never make virtual functions public(!)

This topic is 4841 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

I was reading Herb Sutter's new book, Exceptional C++ style, and found one of the Items on class design very interesting. He suggests that for good design you would by default make virtual functions private. That would be by default. If a derived class really required access to the base virtual function then you could make it protected instead. But only if the design called for it. And you would never make the virtual function public. All public functions would be non-virtual. There seems to be a lot of merit to this. I was wondering whether anyone else had read it and been struck by how much at odds this is with the 'normal' way of doing things, or am I just slow to catch on? Edit: I think I've found the article that the Item was originally based upon. [Edited by - petewood on September 13, 2004 8:51:18 AM]

Share this post


Link to post
Share on other sites
Quote:
Original post by antareus
Is he saying that you should use the template method if the public interface needs to make use of virtual functions?


Yep basically, i've read this aswell.

I think the conclusion to this comes from that fact that we usually encapsulate the variance/variant/variation provide an invariant interface, usually applied when data implementation varies. So i guess the same can be said for logic implementation, if you declare a virtual method in a type then implementation varies on sub-types there for it should also be encapsulated and provide an invariant inteface if needed.

Share this post


Link to post
Share on other sites
Quote:
Original post by antareus
Is he saying that you should use the template method if the public interface needs to make use of virtual functions?


The template method pattern would use protected virtual functions. But if you're not using that pattern or similar then you are advised to make them private.

Unless it's a destructor of course, otherwise you wouldn't be able to create any objects.

Share this post


Link to post
Share on other sites
I don't see why it would be necessary for Objective-C, Java or even C#.
I see this pattern basically as a work-around for C++'s lack of a native interface language construct.
Then again, I might be wrong [smile].

Share this post


Link to post
Share on other sites
So, using the classic Shape example, it is suggested that the following class definitions are used:

class Shape
{
public:
void Draw (void)
{
draw_the_shape ();
}

private:
virtual void draw_the_shape (void) = 0;
}

class Rect : public Shape
{
private:
virtual void draw_the_shape (void)
{
// draw a rectangle
}
}

That would be less efficent than using a public virtual function as it has a second layer of indirection.

Skizz

Share this post


Link to post
Share on other sites
Quote:
Original post by petewood
I was reading Herb Sutter's new book, Exceptional C++ style, and found one of the Items on class design very interesting.

He suggests that for good design you would by default make virtual functions private.

(*snip*)


Hi,

Can you quote the rationale of this particular point ? I'm really interested since I can't see why it is so good (and, you are right, I do not own the book - but I asked my employer to buy it some weeks ago). When I want to have an animal screaming, why should I avoid to make the "scream" method public ? It sounds weird to me :|.

Regards,

(edit : posted that at the same time than Skizz. Can't be just a matter of efficiency - I believe micro-code efficiency is not the goal of such a rather important design consideration. And (from snk_kid) I don't see the point of having a constant interface without virtual functions - If my animal finnaly don't scream then what's the point of having a "scream" method, either virtual or non-virtual ? - Yours,).

Share this post


Link to post
Share on other sites
Quote:
Original post by darookie
I don't see why it would be necessary for Objective-C, Java or even C#.
I see this pattern basically as a work-around for C++'s lack of a native interface language construct.
Then again, I might be wrong [smile].


Its not a pattern (unless your talking about template method) what Herb's talking about i think is more of an observation.

If you really wonted to do the same in java then you would have to use an abstract class instead, also you can write interface types in c++ e.g.


template < typename T >
struct Clonable {

virtual T* clone(const T&) = 0;
virtual ~Cloneable() = 0;
};

template < typename T >
Cloneable<T>::~Cloneable() {}


its just that the compiler is not gonna say anything if you do give a default implementation.

Share this post


Link to post
Share on other sites
Quote:
Original post by petewood
I was reading Herb Sutter's new book, Exceptional C++ style, and found one of the Items on class design very interesting.

He suggests that for good design you would by default make virtual functions private. That would be by default. If a derived class really required access to the base virtual function then you could make it protected instead. But only if the design called for it. And you would never make the virtual function public. All public functions would be non-virtual.

There seems to be a lot of merit to this. I was wondering whether anyone else had read it and been struck by how much at odds this is with the 'normal' way of doing things, or am I just slow to catch on?


I am more interested in why he is suggesting this (His thinking behind it, why he consider it to be good style?) Personally, I consider it good style to hide anything that users of that class have no use for.

Share this post


Link to post
Share on other sites
Quote:
Original post by Skizz
That would be less efficent than using a public virtual function as it has a second layer of indirection.


not if you wack on compiler optimizations, its definiately inlineable but i don't think its worth doing alot except for using it incongunction with template method pattern.

Share this post


Link to post
Share on other sites
The rationale is that virtual functions can be overridden to do anything at all, including a call to crash_program() or fire_nuclear_missiles_at_china() or whatever. This can be a problem if you have an interface to add arbitrary derived class instances to a manager class and have the manager call virtual functions on them.

Not that I can see how making a public interface that just calls a virtual function makes that any safer.

Share this post


Link to post
Share on other sites
Quote:
Original post by Emmanuel Deloget
And (from snk_kid) I don't see the point of having a constant interface without virtual functions - If my animal finnaly don't scream then what's the point of having a "scream" method, either virtual or non-virtual ? - Yours,).


I think you've miss-understood me, for one i'm not for or against it i was giving a reason why Herb would suggest to do this.

Share this post


Link to post
Share on other sites
Quote:
Original post by Paradigm Shifter
The rationale is that virtual functions can be overridden to do anything at all, including a call to crash_program() or fire_nuclear_missiles_at_china() or whatever. This can be a problem if you have an interface to add arbitrary derived class instances to a manager class and have the manager call virtual functions on them.

Not that I can see how making a public interface that just calls a virtual function makes that any safer.


exactly what i was trying to say before:

Quote:
Original post by snk_kid
I think the conclusion to this comes from that fact that we usually encapsulate the variance/variant/variation provide an invariant interface, usually applied when data implementation varies. So i guess the same can be said for logic implementation, if you declare a virtual method in a type then implementation varies on sub-types there for it should also be encapsulated and provide an invariant inteface if needed.


C++ I/O streams lib specifically basic_streambuf does exactly this, all virtual methods are private/protected and most public methods use template method pattern that calls on the private/protected virtual methods that concreate sub-types implement/override suchs as basic_filebuf.

Share this post


Link to post
Share on other sites
Quote:
Original post by MichaelT
I am more interested in why he is suggesting this (His thinking behind it, why he consider it to be good style?) Personally, I consider it good style to hide anything that users of that class have no use for.


Right, you would like to have no dependency on private details:


class ShapeImpl;

//the main class that will be used all over your program
class Shape {
public:
void Draw(DrawingContext&);
void SetImplementation(std::auto_ptr<ShapeImpl>);
private:
ShapeImpl* m_pImpl;
};



//the class which is used to give the shape its characteristics
//likely to be used only in a specific part of your program
//when creating or changing objects
class ShapeImpl {
public:
void Draw(DrawingContext& context) {
DoDraw(context);
}
private:
virtual void DoDraw(DrawingContext&) = 0;
}


Share this post


Link to post
Share on other sites
Quote:

"Traditionally, many programmers were used to writing base classes using public virtual functions to directly and simultaneously specify both the interface and the customizable behavior."

"The problem is that "simultaneously" part, because each virtual function is doing two jobs: It's specifying interface because it's public and therefore directly part of the interface Widget presents to the rest of the world; and it's specifying implementation detail, namely the internally customizable behavior, because it's virtual and therefore provides a hook for derived classes to replace the base implementation of that function (if any). That a public virtual function inherently has two significantly different jobs is a sign that it's not separating concerns well and that we should consider a different approach."


It's not saying no public virtual methods per-sa, it's saying there needs to be two levels. An interface which the whole world sees, and a Template Pattern for customizable behavior. This translates into a set of public virtual methods and a set of protected virtual methods (and private virtuals are generally silly, maybe a private virtual dtor in unusual/rare cases).

If you use a modern implementation of design by contract (CCM, COM), you'll do this anyway. Only the interface will be available to the outside world, and any virtual function in the concrete classes will be hidden even if they are publically defined in a concrete base class.

Share this post


Link to post
Share on other sites
I've had plenty of occasions come up where virtual public functions were infinitely more useful than some cludgy mess. You usually wind up having to use the friend clause or some such.

The most obvious case is when you have a group of drawable classes all derived from the same base; you might have a rendering queue that takes a pointer to the base class and calls the draw function (but each drawable object needs to handle the drawing differently).


Share this post


Link to post
Share on other sites
Quote:
Original post by snk_kid
Its not a pattern (unless your talking about template method) what Herb's talking about i think is more of an observation.

I was indeed referring to the template method.

Share this post


Link to post
Share on other sites
Quote:
Original post by Etnu
I've had plenty of occasions come up where virtual public functions were infinitely more useful than some cludgy mess. You usually wind up having to use the friend clause or some such.

The most obvious case is when you have a group of drawable classes all derived from the same base; you might have a rendering queue that takes a pointer to the base class and calls the draw function (but each drawable object needs to handle the drawing differently).


But in my example you have a Non-Virtual Interface (NVI as Herb calls it), which you store in your containers.

So you have a container of Shape objects rather than a container of Shape*


std::vector<Shape> shapes;
//...
std::for_each(shapes.begin(), shapes.end(), std::mem_fun_ref(&Shape::Draw));

Share this post


Link to post
Share on other sites
If you look at the C++ standard library then you see this pattern everywhere, take the ctype locale class for example, is(...) calls the protected virtual method do_is(...). It's the same with the stream buffer classes too, pubseekoff() calls the virtual protected function seekoff().

Share this post


Link to post
Share on other sites
Quote:
Original post by petewood
Quote:
Original post by MichaelT
I am more interested in why he is suggesting this (His thinking behind it, why he consider it to be good style?) Personally, I consider it good style to hide anything that users of that class have no use for.


Right, you would like to have no dependency on private details:

*** Source Snippet Removed ***
*** Source Snippet Removed ***


Quote:
Original post by petewood
[...]But in my example you have a Non-Virtual Interface (NVI as Herb calls it), which you store in your containers.

So you have a container of Shape objects rather than a container of Shape*

*** Source Snippet Removed ***
But just using the template pattern (if I'm understanding what that means correctly), you'd still store a group of 'Shape's even if you used public virtual methods in the implementation normally:

class ShapeImpl;

//the main class that will be used all over your program
class Shape {
public:
void Draw(DrawingContext&);
void SetImplementation(std::auto_ptr<ShapeImpl>);
private:
ShapeImpl* m_pImpl;
};

class ShapeImpl {
public:
virtual void Draw(DrawingContext&) = 0;
}


A 'shape' is just a different way of saying 'ShapeImpl *' in either case (as far as I can tell). Why have the extra level of redirection in ShapeImpl?

Share this post


Link to post
Share on other sites
Quote:
Original post by Magmai Kai Holmlor
(and private virtuals are generally silly, maybe a private virtual dtor in unusual/rare cases).

Actually, his reasoning behind using private virtuals is very sound. (See guidlines #2 and #3). If the child class should not be directly invoking the method, then there is no reason for it to be able to. Hence why it should be private virtual and not protected virtual.

Share this post


Link to post
Share on other sites
Quote:
Original post by snk_kid
I think you've miss-understood me, for one i'm not for or against it i was giving a reason why Herb would suggest to do this.


No, I wasn't misunderstanding you - I just used your post as a basepoint for futur developments :)

Quote:
Original post by Washu
Actually, his reasoning behind using private virtuals is very sound. (See guidlines #2 and #3). If the child class should not be directly invoking the method, then there is no reason for it to be able to. Hence why it should be private virtual and not protected virtual.


Sounds logical - but why private virtuals then ? As I see it, virtuals are used to allow behavior modifications in a child class. Since the method is private, the child class do not even see it, therefore cannot inherit from it, therefore cannot redefine its behavior. Do I miss something ?

Quote:

"Traditionally, many programmers were used to writing base classes using public virtual functions to directly and simultaneously specify both the interface and the customizable behavior."
[I]and so on... (see Magmai Kai Holmlor's post)[I]


Make sense : the goal seems to be the separation of interface and behavior. I think (I maybe wrong, I'm not a design guru) that this is not that interesting. When you specify the interface of an object, you actually specify how he reacts to external stimulis (methods are refered as messages in the object programming terminologies, and therefore are used to react to messages). If your object provides an abstraction layer (say : an Animal class), it actually tells to the user (this user being a program) what are the messages that can be sent to it, without specifying how it will answer these messages. Intuitivelly, having a non virtual "move" function in an abstract Animal class tells me that any Animal will move exactly the same way. Too bad a bird cannot fly when a cat tries to catch it :P.

As I understand it, Sutter's point may add more complexity to my code without any reward.

Again, I may miss something important here.

Regards,

Share this post


Link to post
Share on other sites
Quote:
Original post by Emmanuel Deloget
Sounds logical - but why private virtuals then ? As I see it, virtuals are used to allow behavior modifications in a child class. Since the method is private, the child class do not even see it, therefore cannot inherit from it, therefore cannot redefine its behavior. Do I miss something ?


if a virtual member function is private sub-types can implement/override but cannot access/use the method.

Share this post


Link to post
Share on other sites

This topic is 4841 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

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