7 replies to this topic

[Nick]

Hi all,

I'm trying to refactor some code but I'm bumping into some practical problems...

Basically I'm trying to do this:

class Interface
{
public:
  virtual void f1() = 0;
  virtual void f2() = 0;
};
 
class Helper : public Interface
{
public:
  void formula()
  {
    f1();
    f2();
    f1();
  }
};
 
class Implementation : public Interface
{
public:
  void f1()
  {
    printf("f1()\n");
  }
 
  void f2()
  {
    printf("f2()\n");
  }
};
 
class Concrete : public Implementation, public Helper
{
public:
  void concrete()
  {
    f1();
    formula();
    f2();
  }
};

If you try to compile this you'll get errors like: " ambiguous access of 'f1' in 'Concrete', could be the 'f1' in base 'Implementation::f1' or the 'f1' in base 'Interface::f1'". Clearly though, there's only one implementation of these functions so in theory it should work.

Ok, it might not be clear what I'm trying to achieve here... In the actual code there will be multiple Helper classes, and multiple Concrete classes. The Helper classes actually merely store a 'sequence of function calls'. They are not supposed to know about any Implementation of these functions, so they just use the Interface. The Concrete classes are the actual end products. They are supposed to have access to the Implementation's functions, and can use the Helper's functions to fulfill their task.

Of course it could all work without this whole hierarchy, just by letting the Concrete classes use the Implementation directly and substituting the function call sequences of the helper classes. However, this would result in huge Concrete classes, and there wouldn't be any code reuse between different Concrete classes.

Still confused why I want to do this? It's intended to be used with SoftWire, my run-time code generator. The Implementation class would be SoftWire's CodeGenerator class, which implements -thousands- of so-called 'run-time intrinsics'. The Helper classes correspond to frequently used code sequences. For example, the SSE code to compute a dot product, or the implementation of a for loop.

Any ideas would be greatly appreciated, because it could mean an imporant step forward for SoftWire. The low-level things have been complete for a while now, but the high-level structure is seriously lacking in functionality to start creating something useful with it, in a convenient way. The goal is to provide 'libraries' of Helper classes so you can build a complete application making use of dynamic code generation, with a minimum (if any) knowledge of assembly language.

All the best,

Nick

[bramz]

There's more than one f1 and f2 since you're using MI. You're class hiearchy actually looks like this (forgive me my ASCII art :)):

Interface     Interface
   |        |
Implementation  Helper
       |  |
      Concrete

So, if you try to call f1 from Concrete, it doesn't know if it has to use f1 from Implementation side or f1 from Helper side ... It doesn't matter f1 and f2 are only implemented in the Implemetation side. In fact, it shouldn't be able to compile because of the pure virtual f1 and f2 on the Helper side.

In short: you're suffering the dreaded diamond syndrom. You're class hiearchy is supposed to look like a diamond, but it isn't. The trick is to use virtual inheritance. Try to add the keyword virtual when deriving from Interface:

class Helper : public virtual Interface;
class Implementation : public virtual Interface;

DISCLAIMER: I don't often use MI myself, and I can't remember I ever used it with the diamond structure before. So it might be I'm missing some details myself. But you get the idea :)

For more information, I gladly refer to the C++ FAQ Lite: http://www.parashift.com/c++-faq-lite/mult...nheritance.html

I hope this helps,
Greetz,
Bramz

PS: I know a lot of people think MI was the biggest mistake in the universe, but I disagree. It's a tool that has to be used with care, and to be avoided if not necessary. But if you need it, then you're damn happy you can.

[Nick]

Thanks Bramz, that works great!

I still get warnings though: 'Concrete' : inherits 'Implementation::f1' via dominance. I guess I can safely disable it?

bramz said:

PS: I know a lot of people think MI was the biggest mistake in the universe, but I disagree. It's a tool that has to be used with care, and to be avoided if not necessary. But if you need it, then you're damn happy you can.

Indeed, I wouldn't know how to do this with single inheritance. :excl: If someone does, please let me know!

[bramz]

Btw, do you want all of f1(), f2(), formula() en concrete() to be visible (and easily usable) from outside Concrete?

If not, i think there might be better solutions than this huge MI tree.

Bramz

[bramz]

Nick said:

Thanks Bramz, that works great!

I still get warnings though: 'Concrete' : inherits 'Implementation::f1' via dominance. I guess I can safely disable it?

Well, think of what happens if another base class implements f1 as well ... Which one does Concrete have to use?

The problem is that you have concrete classes above the join class. Your join class is Concrete (maybe this isn't such a great name after all ;) ) and the concrete class above it is Implementation. Better is to make sure the all classes above the join class are abstract. Then derive an concrete class from it that finally implements f1 and f2 (and thus you get rid of Implementation). The bonus is (a) there's only one entry point for f1 to use (the one in the virtual base class thing) and (b) there's only one implementation to look for (the one in the join class). So gone is ambiguity.

As following:

class Interface
{
public:
  virtual void f1() = 0;
  virtual void f2() = 0;
};
 
class Helper1 : public virtual Interface
{
public:
  void formula()
  {
    f1();
    f2();
    f1();
  }
};
 
class Helper2 : public virtual Interface
{
public:
  void formula()
  {
    f1();
    f2();
    f1();
  }
};

class Concrete: public Helper1, public Helper2
{
public:
  void concrete()
  {
    f1();
    formula();
    f2();
  }

  void f1()
  {
    printf("f1()\n");
  }
 
  void f2()
  {
    printf("f2()\n");
  }
};

If you want to use the same f1 and f2 on many different Joins, you can easily go templated using a policy

class Implementation
{
public:
  void doF1()
  {
    printf("f1()\n");
  }
 
  void doF1()
  {
    printf("f2()\n");
  }
};

template <typename Implementation>
class Concrete: public JoinType
{
  Implementation implementation_;
public:
  void f1()
  {
    implementation_.doF1();
  }
 
  void f2()
  {
    implementation_.doF2();
  }
};

or using static implementation functions:

class Implementation
{
public:
  static void doF1()
  {
    printf("f1()\n");
  }
 
  static void doF1()
  {
    printf("f2()\n");
  }
};

template <typename Implementation>
class Concrete: public JoinType
{
public:
  void f1()
  {
    Implementation::doF1();
  }
 
  void f2()
  {
    Implementation::doF2();
  }
};

Or even by keeping the Join class abstract and deriving a concrete implementation from it:

class Join: public Helper1, Helper2
{
};

class Concrete: public Join
{
  void f1()
  {
    printf("f1()\n");
  }
 
  void f2()
  {
    printf("f2()\n");
  }
};

or even getting that templated to use different Join classes with same implementation:

template <typename JoinType>
class Concrete: public JoinType
{
  void f1()
  {
    printf("f1()\n");
  }
 
  void f2()
  {
    printf("f2()\n");
  }
};

You see, plenty of possibilities :)

[bramz]

One more possibility which would probably have my vote: keep a pointer to a concrete Implementation in your Interface class, so you can mix and match M different Implementation at run-time:

class AbstractImplementation
{
public:
  virtual void f1() = 0;
  virtual void f2() = 0;
}

class ConcreteImplementation: public AbstractImplementation
{
  void f1() { printf("f1()\n"); }
  void f2() { printf("f2()\n"); }
};

class BuilderBase
{
protected:
  AbstractImplementation* impl_;
};

class Helper1: public virtual BuilderBase
{
public:
  void formula()
  {
    impl_->f1();
    impl_->f2();
    impl_->f1();
  }
};

class Builder: public Helper1, public Helper2:
{
public:
   Builder(AbstractImplementation* impl) { impl_ = impl; }
};
   
ConcreteImplementation impl;
Builder builder(&impl);

[Nick]

bramz said:

Better is to make sure the all classes above the join class are abstract. Then derive an concrete class from it that finally implements f1 and f2 (and thus you get rid of Implementation). The bonus is (a) there's only one entry point for f1 to use (the one in the virtual base class thing) and (b) there's only one implementation to look for (the one in the join class). So gone is ambiguity.

That's not really an option because the 'Implementation' class is SoftWire's CodeGenerator class, and the Concrete class is a user-implemented class, for example a scripting engine. It's not the responsability of the scripting engine to implement SoftWire's run-time intrinsics...

Or in other words: The Implementation class is unique, but there can be many Concrete classes. Without the virtually inherited Implementation class, all Concrete classes would have to implement the Interface in the exact same way. Not good for code reuse and very bug-prone when changes have to be made to the implementation!

[bramz]

Nick said:

bramz said:

Better is to make sure the all classes above the join class are abstract. Then derive an concrete class from it that finally implements f1 and f2 (and thus you get rid of Implementation). The bonus is (a) there's only one entry point for f1 to use (the one in the virtual base class thing) and (b) there's only one implementation to look for (the one in the join class). So gone is ambiguity.

That's not really an option because the 'Implementation' class is SoftWire's CodeGenerator class, and the Concrete class is a user-implemented class, for example a scripting engine. It's not the responsability of the scripting engine to implement SoftWire's run-time intrinsics...

Or in other words: The Implementation class is unique, but there can be many Concrete classes. Without the virtually inherited Implementation class, all Concrete classes would have to implement the Interface in the exact same way. Not good for code reuse and very bug-prone when changes have to be made to the implementation!

I don't think I'm following ... why does all this has to be polymorphic? As I see it, Concrete class is only using Implementation. It doesn't look like it has to be an Implementation, or has to be an Interface.

Bramz