views:

346

answers:

4

I've been doing some mocking with RhinoMocks and it requires that mocked methods be made virtual. This is fine except we have a custom framework which contains the methods that I want to mock which are currently not marked as virtual.

I can't forsee any problem with making these methods virtual but I was wondering what are some potential dangers of making methods virtual that I should look out for?

+4  A: 
  • If you have users that override your virtual methods you can't seal them again without breaking code.
  • Any virtual methods you call from the constructor may fall down to derived implementations and if they don't call the base method and the constructor depends on it, the object may be in an invalid state
Mark Cidade
+2  A: 

Ayende has a nice treatment of how virtual methods work:

http://ayende.com/Blog/archive/2007/01/05/HowVirtualMethodsWork.aspx

Kev
+1  A: 

Virtual methods can be potentially dangerous (or in best case cause strange behaviour) if you call them during the destruction of an inherited object.

Let's say you have the following classes:

class foo {
public:
   foo();
   ~foo() { Cleanup() };
   virtual void Cleanup();
}

class bar : public foo {
public:
   bar()
      :foo() {};
   ~bar();
   virtual void Cleanup()
   {
      foo::Cleanup();

      // Do some special bar() related cleanup now...
   }
}

Let's say you create an object of type bar(), which is actually a foo() based on the inheritance hierarchy. When you delete the bar() object, the bar() destructor gets invoked first, followed by the foo() destructor. In an attempt to minimise calls to the Cleanup() routine, the call to this has been put in the foo() destructor. However, when this is called, the bar() object has already been destroyed, which means foo::Cleanup() won't exist in the vtable, and it will call the bar::Cleanup() method instead.

This could result in strange behaviour, like memory leaks, or inconsistent system state which could be a ticking time bomb waiting to cause problems further down the line.

I have been bitten by this sort of strange behaviour in the past, so it's certainly something to look out for.

LeopardSkinPillBoxHat
No, C++ guarantees non-virtual behaviour in the constructor and destructor. The compiler should have disallowed you to call Cleanup() in foo's destructor, because it's not implemented in class foo, and it's not a virtual call.
Chris Jester-Young
For indirect calls to Cleanup() in the dtor (e.g., dtor calls a non-virtual function that then calls virtual Cleanup()), then you should get a "pure virtual call" error. (Mind you, the behaviour is undefined at this stage, but Visual C++ will advise you of a pure virtual call at runtime.)
Chris Jester-Young
+7  A: 

Actually it can be very problematic if the method is not designed to be overridden and someone overrides it. In particular, never call a virtual method from a constructor. Consider:

class Base {
    public Base() {
       InitializeComponent();
    }
    protected virtual void InitializeComponent() {
        ...
    }
}

class Derived : Base {
    private Button button1;
    public Derived() : base() {
        button1 = new Button();
    }
    protected override void InitializeComponent() {
        button1.Text = "I'm gonna throw a null reference exception"
    }
}

The Derived class may not be aware that the virtual method call will result in its InitializeComponent method being called before a single line of its own constructor has run.