Does the Non-Virtual Interface idiom not apply to pure virtual functions?

331 Views Asked by At

I used to write interface classes like this:

class SomeInterface
{
public:
    virtual void doThings() = 0;
    virtual void run() = 0;
};

class OtherInterface
{
public:
    virtual void doStuff() = 0;
    virtual void run() = 0;
};

Which meant that I could easily inherit several interfaces and use it like this

class ConcreteClass : public SomeInterface, public OtherInterface
{
public:
    virtual void doThings() { std::cout << "do things" << std::endl; }
    virtual void doStuff() { std::cout << "do stuff" << std::endl; }
    virtual void run() { std::cout << "do run" << std::endl; }
};

int main()
{
    ConcreteClass myObject;
    myObject.run();
    return 0;
}

But I read recently about the Non-Virtual Interface idiom (stuff like this http://www.gotw.ca/publications/mill18.htm), and following those guidelines I should instead write my interfaces like this:

class SomeInterface
{
public:
    void doThings() { doThingsImplementation(); }
    void run() { runImplementation(); }
private:
    virtual void doThingsImplementation() = 0;
    virtual void runImplementation() = 0;
};

class OtherInterface
{
public:
    void doStuff() { doStuffImplementation(); }
    void run() { runImplementation(); }
private:
    virtual void doStuffImplementation() = 0;
    virtual void runImplementation() = 0;
};

Which means the rest of the code now looks like this

class ConcreteClass : public SomeInterface, public OtherInterface
{
private:
    virtual void doThingsImplementation() { cout << "do things" << endl; }
    virtual void doStuffImplementation() { cout << "do stuff" << endl; }
    virtual void runImplementation() { cout << "run" << endl; }
};

int main()
{
    ConcreteClass myObject;
    myObject.run();
    return 0;
}

...except that now it doesn't compile because the call to run() has become ambiguous (error: request for member ‘run’ is ambiguous). What used to be a harmless overlap in the interfaces now results in name collision.

Rereading the argument for the Non-Virtual Interface idiom, I get the feeling that the issue is because I'm applying it to virtual functions that are pure virtual functions. Is that it? Should I only use the NVI idiom for non-pure virtual functions?

Edit 1: Someone remarked that the run() function could be in its own interface class. That actually makes a lot of sense: either both run() have the same meaning and the interface should be factored, or they have different meaning and we want the compiler to say things are ambiguous. This still leaves us with a diamond inheritance issue when using NVI that isn't there with pure virtual functions

Edit 2: It turns out that, if I factor out run() as a pure virtual function in its own interface class from which both SomeInterface and OtherInterface inherit, things don't actually behave as well as I thought. If I introduce CombinedInterface (inheriting from both interface classes) and have ConcreteClass inherit from that, I can call run() on myObject but I cannot use it polymorphically from a CombinedInterface* (again, the call is considered ambiguous).

Conclusion: My takeaway here is that 1) Interfaces should either not overlap, or be factored out. and 2) Inheritance between interface classes should always be virtual

2

There are 2 best solutions below

7
n. m. could be an AI On

You just need to shadow the conflicting name in the derived class.

class ConcreteClass : public SomeInterface, public OtherInterface
{
  public:
    void run() { runImplementation(); }
  // rest of the class

Doesn't shadowing defeat the purpose of NVI?

Oh, but what is the purpose of NVI? It's not there for its own sake, we usually add pre- and post-operations in the non-virtual part as well. Now if the pre- and post-operations are different in SomeInterface and OtherInterface, then you have choice but combine them somehow in the derived class.

class ConcreteClass : public SomeInterface, public OtherInterface
{
  public:
   void run() {
     SomeInterface::preRun();
     OtherInterface::preRun();
     runImplementation() // etc      
 }

But if they are the same to begin with, then perhaps run should be in its own interface, inherited virtually by both SomeInterface and OtherInterface. (Virtually because otherwise there are still two copies of run, each one in its own base subobject).

4
Caleth On

You need to specify which interface's run you want to call, because in general they do different things.

int main()
{
    ConcreteClass myObject;
    myObject.SomeInterface::run();
    // or
    myObject.OtherInterface::run();
    return 0;
}