How to ensure that virtual method calls get propag

2019-03-09 23:20发布

问题:

One very common mistake with class hierarchies is to specify a method in a base class as being virtual, in order for all overrides in the inheritance chain to do some work, and forgetting to propagate the call on to base implementations.

Example scenario

class Container
{
public:
  virtual void PrepareForInsertion(ObjectToInsert* pObject)
  {
    // Nothing to do here
  }
};

class SpecializedContainer : public Container
{
protected:
  virtual void PrepareForInsertion(ObjectToInsert* pObject)
  {
    // Set some property of pObject and pass on.
    Container::PrepareForInsertion(pObject);
  }
};

class MoreSpecializedContainer : public SpecializedContainer
{
protected:
  virtual void PrepareForInsertion(ObjectToInsert* pObject)
  {
    // Oops, forgot to propagate!
  }
};

My question is: is there a good way/pattern to ensure that the base implementation always gets called at the end of the call chain?

I know of two methods to do this.

Method 1

You can use a member variable as a flag, set it to the correct value in the base implementation of the virtual method, and check its value after the call. This requires to use a public non-virtual method as interface for the clients, and making the virtual method protected (which is actually a good thing to do), but it requires the use of a member variable specifically for this purpose (which needs to be mutable if the virtual method must be const).

class Container
{
public:
  void PrepareForInsertion(ObjectToInsert* pObject)
  {
    m_callChainCorrect = false;
    PrepareForInsertionImpl(pObject);
    assert(m_callChainCorrect);
  }

protected:
  virtual void PrepareForInsertionImpl(ObjectToInsert* pObject)
  {
    m_callChainCorrect = true;
  }

private:
  bool m_callChainCorrect;
};

class SpecializedContainer : public Container
{
protected:
  virtual void PrepareForInsertionImpl(ObjectToInsert* pObject)
  {
    // Do something and pass on
    Container::PrepareForInsertionImpl(pObject);
  }
};

Method 2

The other way to do it is to replace the member variable with an opaque "cookie" parameter and do the same thing:

class Container
{
public:
  void PrepareForInsertion(ObjectToInsert* pObject)
  {
    bool callChainCorrect = false;
    PrepareForInsertionImpl(pObject, &callChainCorrect);
    assert(callChainCorrect);
  }

protected:
  virtual void PrepareForInsertionImpl(ObjectToInsert* pObject, void* pCookie)
  {
    *reinrepret_cast<bool*>(pCookie) = true;
  }
};

class SpecializedContainer : public Container
{
protected:
  virtual void PrepareForInsertionImpl(ObjectToInsert* pObject, void* pCookie)
  {
    // Do something and pass on
    Container::PrepareForInsertionImpl(pObject, pCookie);
  }
};

This approach is inferior to the first one in my opinion, but it does avoid the use of a dedicated member variable.

What other possibilities are there?

回答1:

You've come up with some clever ways to do this, at (as you acknowledge) the cost of bloating the class and adding code that addresses not the object's responsibilities but programmer deficiences.

The real answer is not to do this at runtime. This is a programmer error, not a runtime error.

Do it at compile time: use a language construct if the language supports it, or use a Pattern the enforces it (e.g,, Template Method), or make your compilation dependent on tests passing, and set up tests to enforce it.

Or, if failing to propagate causes the derived class to fail, let it fail, with an exception message that informs the author of the derived class that he failed to use the base class correctly.



回答2:

What you are looking for is simply the Non-Virtual Interface pattern.

It's similar to what you are doing here, but the base class implementation is guaranteed to be called because it's the only implementation that can be called. It eliminates the clutter that your examples above require. And the call through the base class is automatic, so derived versions don't need to make an explicit call.

Google "Non-Virtual Interface" for details.

Edit: After looking up "Template Method Pattern", I see that it's another name for Non-Virtual Interface. I've never heard it referred to by the name before (I'm not exactly a card-carrying member of the GoF fan club). Personally, I prefer the name Non-Virtual Interface because the name itself actually describes what the pattern is.

Edit Again: Here's the NVI way of doing this:

class Container
{
public:
  void PrepareForInsertion(ObjectToInsert* pObject)
  {
    PrepareForInsertionImpl(pObject);

    // If you put some base class implementation code here, then you get
    // the same effect you'd get if the derived class called the base class
    // implementation when it's finished.
    //
    // You can also add implementation code in this function before the call
    // to PrepareForInsertionImpl, if you want.
  }

private:
  virtual void PrepareForInsertionImpl(ObjectToInsert* pObject) = 0;
};

class SpecializedContainer : public Container
{
private:
  virtual void PrepareForInsertionImpl(ObjectToInsert* pObject)
  {
    // Do something and return to the base class implementation.
  }
};


回答3:

When there's only one level of inheritance you can use the template method pattern where the public interface is non-virtual and calls a virtual implementation function. Then the base's logic goes in the public function which is assured to be called.

If you have more than one level of inheritance and want each class to call its base class then you can still use the template method pattern, but with a twist, make the return value of the virtual function only constructable by base so derived will be forced to call the base implementation in order to return a value (enforced at compile time).

This doesn't enforce that each class calls its direct base class, it may skip a level (I can't think of a good way to enforce that) but it does force the programmer to make a conscious decision, in other words it works against inattentiveness not malice.

class base {
protected:
    class remember_to_call_base {
        friend base;
        remember_to_call_base() {} 
    };

    virtual remember_to_call_base do_foo()  { 
        /* do common stuff */ 
        return remember_to_call_base(); 
    }

    remember_to_call_base base_impl_not_needed() { 
        // allow opting out from calling base::do_foo (optional)
        return remember_to_call_base();
    }

public:
    void foo() {
        do_foo();
    }
};

class derived : public base  {

    remember_to_call_base do_foo()  { 
        /* do specific stuff */
        return base::do_foo(); 
    }
};

If you need the public (non virtual) function to return a value the inner virtual one should return std::pair<return-type, remember_to_call_base>.


Things to note:

  1. remember_to_call_base has an explicit constructor declared private so only its friend (in this case base) can create a new instance of this class.
  2. remember_to_call_base doesn't have an explicitly defined copy constructor so the compiler will create one with public accessibility which allows returning it by value from the base implementation.
  3. remember_to_call_base is declared in the protected section of base, if it was in the private section derived won't be able to reference it at all.


回答4:

A completely different approach would be to register functors. Derived classes would register some function (or member function) with the base class while in the derived class constructor. When the actual function is called by the client it is a base class function which then iterates through the registered functions. This scales to many levels of inheritance, each derived class only has to be concerned with its own function.



回答5:

Look at the template method pattern. (The basic idea is that you don't have to call the base class method anymore.)



回答6:

One way out is to not use virtual methods at all, but instead to allow the user to register callbacks, and call those before doing the work of prepareForInsertion. This way it becomes impossible to make that mistake since it is the base class that makes sure that both the callbacks and the normal processing happen. You can end up with a lot of callbacks if you want this behaviour for a lot of functions. If you really do use that pattern so much, you might want to look into tools like AspectJ (or whatever the C# equivalent is) that can automate this sort of thing.



回答7:

If you found out you can hide virtual function and make interface non-virtual, try instead of checking if other users did call your function just call it by yourself. If your base code should be called at the end, it would look like this:

class Container
{
public:
  void PrepareForInsertion(ObjectToInsert* pObject)
  {
    PrepareForInsertionImpl(pObject);
    doBasePreparing(pObject);
  }

protected:
  virtual void PrepareForInsertionImpl(ObjectToInsert* pObject)
  {
    // nothing to do
  }

private:
  void doBasePreparing(ObjectToInsert* pObject)
  {
    // put here your code from Container::PrepareForInsertionImpl
  }
};