6

What is a good way to unscramble the circular inheritance here?

class Node {
   // ...
public:
   list<Node*> neighbors() { /* ... */ }
   void update() { }
}

template<class NodeType>
class HasImportance : public virtual NodeType {
   double m_importance = 0.0;
public:
   void receive_importance(double imp) { /* ... */ }
   void give_importance() {
      for (auto neighbor : this->neighbors())
         neighbor->receive_importance(m_importance /* ... */);
   }
};

class TrafficLight : public HasImportance<TrafficLight>, virtual Node {
public:
   list<TrafficLight*> neighbors() { ... }
   void update() { give_importance(); /* ... */ }
};

It fails (gcc 4.7.0) because TrafficLight is an incomplete type when HasImportance tries to inherit from it.

The real problem is that HasImportance needs to know the type returned by neighbors(). If HasImportance inherits from Node, then it thinks neighbors() returns a list of Node*, not TrafficLight*, and consequently doesn't know that it can call receive_importance() on the items. Similar problem if HasImportance doesn't inherit at all.

BTW, what I'm trying to do is make a few mix-ins to help define a variety of different kinds of graphs easily and to unit-test each mix-in separately. For example, I should be able to define the node class for a graph of traffic lights by just writing something like class TrafficLight : public HasImportance, HasState<3>, virtual Node { }.

I've come up with three ways to solve this, but all seem ugly. (1) static_cast<NodeType*>. (2) TrafficLight passes its this to HasImportance in its constructor. This way, HasImportance doesn't need to inherit at all; it just stores a pointer to (ahem) itself, and the template parameter provides the type of the pointer. (3) Make Node a class template, like this: 

template<class NodeType>
class Node {
public:
   list<NodeType*> neighbors() { /* ... */ }
}

class TrafficLight : public HasImportance<Node<TrafficLight>> { /* ... */ }

That compiles and it doesn't introduce a gratuitous copy of this, but it seems…a little too curious.

Is there a code smell here? Should I approach these graphs in a completely different way?

Ben Kovitz
  • 4,920
  • 1
  • 22
  • 50
  • 11
    Using `static_cast(this)` is *normal* in CRTP. – kennytm Jul 03 '12 at 12:52
  • @KennyTM: I would even go so far and say that this is the key in using the CRTP – PlasmaHH Jul 03 '12 at 13:29
  • Thanks. I cringe at using static_cast, because it seems like I'm ignoring a sign (a "smell") that something deeper is wrong. If it's "normal" in CRTP, I guess I won't resist so much. This is my first CRTP. Can you tell? :) – Ben Kovitz Jul 11 '12 at 15:39

1 Answers1

1

(3) but a bit differently.

template <class NodeType>
class Node { ... };

template<class NodeType>
class HasImportance : public virtual Node<NodeType> { ... };

class TrafficLight : public HasImportance<TrafficLight> { ... };

Looks entirely straightforward to me, not more curious than the CRTP itself.

n. m. could be an AI
  • 112,515
  • 14
  • 128
  • 243
  • Thanks! I like this a lot better even though it's a small difference. The "compile-time interface" to the mix-ins is now simple and pretty immune to changes elsewhere in the code, unlike my version. – Ben Kovitz Jul 11 '12 at 15:53