Should too many virtual functions raise a red flag?

[awhite1159]

I am designing a network backup simulator which during my initial design I defined an abstract base called 'datamover'. From this I derive several classes which are network node, network, NIC (For nodes with multiple network interfaces), disk, tape library, etc. What I have found thus far having only defined the network type datamovers is that although a network and network nodes may have similarities networks also have alot of unique ways of doing things. I'm piling on the virtual functions since when I call these objects through my handle class, they need to be polymorphic. Is this a red flag that the network should not be based on datamover? How many virtual functions are too many?

[satm2008]

Not sure of the implications of too too many virtual functions in an abstract flass, but the base class would grealy rely on its derived clases for its virtual method implementations.

And a pure virtual method in a class would make it abstract class, which in turn can not be instantiated. Only its derived classes can be instantiated defining the derived virtual methods functionality.

And one more thing to note is that, each virtual function creates an entry in "virtual nmethod table" of the class meaning increasing size of the class's object.

Coming to performance, each call to a virtual method (during the runtime) requires at least one extra indexed dereference (to virtual method table) to check which one is what. This leads to an extra overhead during the runtime execution.
If it is a non-virtual call then it would be a quick jump to compiled-in pointer object and quicker in terms of performance.
Hence the smaller the size of vtable (ie, virtual method table) higher the performance.

Too many virtual methods, too slow to check and dispatch right method in runtime.

I would say, you better check design alternatives (like storing nodes in binary search tree etc) and reduce the vtable side.

And no idea of maximum number of virtual methods, as I never used that so many :)

Good luck :)

[awhite1159]

Thanks for responding satm2008.

I'm learning that sound design is critical. I now believe that virtual functions should only exist for things that are common to their derived classes but behave differently. Even though objects may have many things in common, they may also have things that are not in common. My original approach was to lump everything into the abstract base class and if it did not apply to a subclass, just have it call an empty function in the subclass or throw an exception. Doesn't that seem sloppy?

I wanted to have a common handle to the base class so I could treat all the objects like DataMovers. I have now changed the design to instead have two different handles which point to the immediate subclass.

Are these the common issues that have to be thought out and dealt with? Am I on the right track?

[Faldrax]

I think you are certainly on the right track when you say sound design is critical:-)

You said in your original post that you needed to call the methods from the base class handle - presumably the logic of your code is (or will be) such that you only try to call a particular method if it is suitable to the object in question?
If so, you could keep the base class to hold just the common methods, and then cast the handle to the approriate descendant when you needed to call a method that was not in the base class.

[satm2008]

In that case, you better off with regula inheritence than an abstract. As you may be already aware of, an abstract class can NOT be instantiated as it does have only a skeleton (methods) and no flesh (definition) for the members has it has declared in.

Same as Faldrax thought, you better remove the abstractness and define the basic, which is common across where it is missing in the derived, functionality.
So that if derived does not have its own version of it, then the base's comes in take care of it.
How does that sound?

[ropez]

>> each virtual function creates an entry in "virtual nmethod table" of the class meaning increasing size of the class's object.

This is not correct! Each virtual function adds an entry to the table, but there's only one table for each class. It doesn't affect the size of an object! Each object contains a pointer to the vtable, which is exactly the same if there are one or a hundred thousand virtual functions.

The number of virtual functions will not have a significant effect on memory usage, unless you have millions.

There's no need to try and minimize the number of virtual functions. You should try to create the best possible design. If that means you need many virtual functions, you shouldn't care about it.

If there's anything you should try to avoid in general, it's casting down from the base class to derived classes. It's much better to use a common interface.

BTW: In Java, and many other languages all methods are virtual functions.

[satm2008]

Sorry, my typo.

I meant the overhead not the object size.
I meant that it would increase an overhead on the class performance.

[ropez]

The overhead of virtual functions is really microscopic. If you implement something like a sort function that sorts a list of several million integers, and use virtual functions for basic operations like comparing and swapping elements, you'll probably get increased executable time by a couple of seconds. This is why crucial stuff like std::iterators don't use virtual functions.

However, in most cases the overhead of calling a function is nothing compared to the function itself.

BTW, if you're using iostream, there's probably a lot of virtual function calls going on anyway.

[satm2008]

Check the link

http://en.wikipedia.org/wiki/Virtual_table
and the note under Efficiecy heading

and a full description of it in
http://www.cs.ucsb.edu/~urs/oocsb/papers/oopsla96.pdf


Good luck :)

[ropez]

All the wikipedia article says is that calling a virtual function may be slower than calling non-virtual function. There's no doubt about that! But what I'm saying is that in most programs, there's not much to gain by minimizing the overhead of function calls.

In the article that is referenced from wikipedia, they did the benchmarks on programs that called virtual functions millions of times (when the compiler was instructed to make all calls virtual). Of course, if you have a very small function, and you call it a million times, you could get 50% slowdown.

However, my guess is that in the OP's program, most functions are relatively complex and are not called often enough to worry about virtual function overhead.

For instance, if a function writes anything to cout, reads/writes to a file or a network socket, contains a loop or does any non-trivial calculations, executing the function itself is probably hundreds or thousands of times slower than the overhead of calling it.

[satm2008]

It is C++, performance is great comparitively.
So I would not bother about the overhead of virtual functions, though I would not write such many many virtual methods.
Even so, I dont think C++ would show a significant change in the performance.

That is what C++ is :)

[awhite1159]

Thanks for all the quick replies. I did consider downcasting but as you mention ropez, I want to avoid doing that for the same reasons. I now have added two subclasses which still remain abstract and have the datamover as the superclass. They are NetObject and NetContainer. Network nodes will inherit from NetObject and Networks will inherit from NetContainer. NetContainers are datamovers which have NetObjects. As I pass messages from NetObject to NetObject which will simulate network packet flow, if they encounter a NetContainer, they will check to see if the NetObject is in its own container. If not, they will pass it through a routing mechanism to another NetContainer.

Even if I never get this to work or become useful in my real job, it is a great mind exercise and alot of fun.

[ropez]

While not having the complete picture of your design, it sounds to me that you have come a long way towards your goal. If you're generally happy with the design, you shouldn't worry too much about compiler/language details such as method invocation overhead. At least not until it's actually causing a problem.

[Faldrax]

ropez,
Why do you say you should avoid downcasting?
You have dynamic_cast to perform type checking for you so the cast will be safe (or return a null pointer).
The alternative is to add dummy methods to your classes

EG You have a class 'vehicle', with a subclass 'car', with subclasses 'manual_car' and 'automatic_car'.
Why is casting a 'vehicle' to 'manual_car' to call 'select_gear' (via a suitable check that the 'vehicle' is a 'manual _car') somgthing to be avoided?

Isn't this part of the point of having class heirarchies? You put the common parts in the base class, then add the specifics in the descendant classes.
If you are forced to put virtual methods in the base class for everythign it quickly becomes unwieldy, and code maintenace becomes a problem.

In the example above, you subsequently require an 'open_sunroof' method for 'cars'. Do you then implement this as a virtual method in the base class, and dummy methods in ALL the descendants ('truck', 'bycycle', etc)?

[ropez]

@Faldrax:

First, if performance is an issue, a dynamic_cast is a very complex operation. I have no actual measurements, but it's probably a lot more costly than calling a virtual function. However, in most cases (as already discussed in this thread), such microscopic details have small impact on performance.

Casting in general, and specially downcasting is often a sign of bad design. In other forums, like news groups I have read, even mentioning dynamic_cast causes controversy. I've read posts saying that they would never ever even consider using such a thing. I'm not that religious though, but it's still something you should try to avoid.

In your example, I assume there is a function or class that wants to call "select_gear". In that case, why doesn't this function have a reference to a 'manual_car' in the first place? If this function operates on generic 'vehicles', it shouldn't have to deal with details on how each car is implemented.

The function probably looks something like this:

void Traffic::handleVehicles() {
  // for each vehicle v
  {
    Car* c = dynamic_cast<Car*>(v);
    if (c) {
      AutomaticCar* ac = dynamic_cast<AutomaticCar*>(c);
      if (ac) {
        ac->selectAutomaticGear();
      }
      ManualCar* mc = dynamic_cast<ManualCar*>(c);
      if (mc) {
        mc->selectGear(number);
      }
      // do other driving stuff with car
    }
    Train* t = dynamic_cast<Train*>(v);
    if (t) {
      // ...
    }
    // more vehicle types...
  }
}

Suppose you add new vehicle types, you need to update all functions that use the class hierarchy like this. Isn't is much better to say:

void Traffic::handleVehicles() {
  // for each vehicle v
  {
    v->drive();
  }
}

I could write more examples, but I hope you get my point.

[Faldrax]

Agreed, dynamic_cast which uses RTTI is certainly slower than calling a virtual method, but as you say, in the main such performance issues are not an issue (poor design can easily outweigh sensible use of RTTI).

Casting can be a sign of bad design - yes, but that does not mean it is intrinsicaly bad design. Any design, to some extent, is a balancing act between competing requirements - performance, maintainablilty, extensibility, implementsion cost, etc. Casting is one option in a design, ignoring this option may well end up forcing a design that is bad in other ways.

...'In that case, why doesn't this function have a reference to a 'manual_car' in the first place?'
But to get a reference to a 'manual_car' (rather than a 'vehicle') from the general list (for example) of 'vehicle' objects you will need to cast the 'vehicle' to a 'manual_car'...

Looking at the final example, yes, it would be better. What you have done is encapsulate the set of different methods of driving a vehicle into a single public method for the base class - the implementaion in the descentant classes will then have the various different private methods called from within their particular implementaion of drive. This would be good design.

But where you have something that does not fit into that model (such as 'open_sunroof') casting may be an option that allows for a tidy way of handling such functionality.

Perhaps we could agree on 'Use casting with care'?

[ropez]

>> But to get a reference to a 'manual_car' (rather than a 'vehicle') from the general list (for example) of 'vehicle' objects you will need to cast the 'vehicle' to a 'manual_car'...

Yes, but I meant that it shouldn't have a general list. If this function handles 'manual_car', it should have a list of 'manual_car'. If it handles generic vehicles, it shouldn't do anything specific to a manual car. If it handles different 'vehicle' types differently, it can have several lists. In fact, it might be a better design not to have this function at all.

I think it's difficult to discuss this without having real examples. But I think in most cases if your code depends on downcasting, then it's possible to get a better solution by using a different design. However it requires a lot of experience to see how this redesign can be done. In general, without actual code, it's impossible to say what's the "best" solution. Although there are some common design patterns that deal directly with these issues. e.g. visitor pattern, double dispatch.

[satm2008]

I agree with Faldrax, though the downcasting comes with a runtime overhead, it would not be called a bad design if used right, a dynamic_cast in the program.
Considering the same example, the base class provides a skeleton for only common functionality and allows the derived to have their own versions of same methods. To handle the code right or call the class-specific functions right, a dynamic_cast would be a right idea, in fact, I would say. A work around for this may be costly.
As a C++ base class pointer recognizes and allows it pointing to its derived class, and a polymorphic method to act dynamically right, a dynamic_cast kind of downcasting is good idea and not at all a sign of bad design. I am absolutely OK with that.

Example,

void handleGear(Vehicle *pV)
{
   if (dynamic_cast<ManualCar *>(pV) ) 
   {
       // …check the manually switched gear and change the speed 
   }
   else if (dynamic_cast<AutomaticCar *>(pV) ) 
   {
       // …check speed and change the gar automatically 
   }
   else if (dynamic_cast<MotorBike *>(pV) ) 
   {
       // …select gear code for a motor bike
   }
   else if (dynamic_cast<BiCycle *>(pV) ) 
   {
       // …select gear code for a bicycle 
   }
   else {
         // throw exception/error
   }
}

Though some overhead is involved in this function, in the above scenario, a check with downcasting is NEEDED and not a bad program .

Good luck :)

[satm2008]

As we all know, the casting is always need to be handled carefully and cautious :)

[ropez]

@satm2008:
Suppose that in your project, you have not one but a hundred functions like the one in your example. Also, suppose they are more complex, each function may have a different set of subclasses that it handles, and there may be more than two levels in the class hierarchy.

Now what happens if you add a new class, or even worse, if you have to change something like split the MotorBike class into two new classes.

Instead of having to add/change/split one vehicle implementation, you need to manually look up every function that uses the vehicle classes, and for each function you need to carefully rewrite the function to handle the change.

For more discussion about this, see:
http://groups.google.com/group/comp.lang.c++/browse_thread/thread/99cdc317f5a66b33/963c795b7c36fbde