Diamond Inheritance and STL

[HGallon]

I have an application where I have visual elements which are a: Moving
or Stationary, and b: Static or Animated

//
using namespace std;

//
class element
{
public:
virtual void Paint (HDC hDC);
};

//
class movingElement : public element
{
public:
void Move ();
};

//
class animatedElement : public element
{
public:
void Update (); // get next image in
// animated sequence
};

//
class movingAnimatedElement : public movingElement,
public animatedElement
{
};

So far, so good. Now I need a container to display all visual elements
sorted by e.g. Z-Order

//
class elementList
{
protected:
list<element*> m_list;

public:
virtual void addElement (element* pEl);
};

Now, when adding all elements into a sorted list

//
void fn ()
{
element el;
movingElement mEl;
animatedElement aEl;
movingAnimatedElement maEl;

elementList elList;

elList.addElement (&el); // ok
elList.addElement (&mEl); // ok
elList.addElement (&aEl); // ok
elList.addElement (&maEl);
Error C2594: 'argument' : ambiguous conversions from 'class
movingAnimatedElement*' to 'class element*'

}

C2594 is defined as "'operator' : ambiguous conversions from 'type1' to
'type1' No conversion from one specified type to the other was more
direct than any other. It may be necessary to define or specify an
explicit conversion."

If I convert maEl to a movingElement, I can't update the animated image
frame; if I convert it to an animatedElement, I can't move it. And I'd
rather not add "virtual void Move ()" to the definition of element and
everything derived from it. Any help, anyone?

 

[Alf P. Steinbach]

* (e-mail address removed):

I have an application where I have visual elements which are a: Moving
or Stationary, and b: Static or Animated

//
using namespace std;

//
class element
{
public:
virtual void Paint (HDC hDC);
};

//
class movingElement : public element
{
public:
void Move ();
};

//
class animatedElement : public element
{
public:
void Update (); // get next image in
// animated sequence
};

//
class movingAnimatedElement : public movingElement,
public animatedElement
{
};

So far, so good.

Well, no. You have two distinct base class objects of type 'element', one
belonging to movingElement and on to animatedElement. So if you try to call
'Paint' on a movingAnimatedElement the compiler will report an ambigious call:
did you mean to call the movingElement Paint or the animatedElement Paint?

And, noting that it doesn't make much sense to call *both* (which could be
arranged in the final bottom level class, but isn't practically meaningful), you
have a design level problem, not just a C++ implementation problem.

Adding "virtual" in the inheritance chain is /not/ a solution of that design
level problem -- for with 'element' a virtual base class you still have the
Paint problem.

Thus, the resolution hinges on what functionality you really ahve in
movingElement and in animatedElement.

I.e., does it really make sense to combine these two via multiple inheritance (I
think not, but could be, depending on what they really are).

Now I need a container to display all visual elements
sorted by e.g. Z-Order

//
class elementList
{
protected:
list<element*> m_list;

public:
virtual void addElement (element* pEl);
};

Now, when adding all elements into a sorted list

//
void fn ()
{
element el;
movingElement mEl;
animatedElement aEl;
movingAnimatedElement maEl;

elementList elList;

elList.addElement (&el); // ok
elList.addElement (&mEl); // ok
elList.addElement (&aEl); // ok
elList.addElement (&maEl);
Error C2594: 'argument' : ambiguous conversions from 'class
movingAnimatedElement*' to 'class element*'

}

C2594 is defined as "'operator' : ambiguous conversions from 'type1' to
'type1' No conversion from one specified type to the other was more
direct than any other. It may be necessary to define or specify an
explicit conversion."

If I convert maEl to a movingElement, I can't update the animated image
frame; if I convert it to an animatedElement, I can't move it. And I'd
rather not add "virtual void Move ()" to the definition of element and
everything derived from it. Any help, anyone?

You need to get concerete about what movingElement and animatedElement are.

For without that you can't solve the design level problem.

It's not a C++ problem, the solution is not "virtual" inheritance, it is a
design level problem, where at first I think it will be helpful for you to focus
on exactly what a Paint call on a movingAnimatedElement should result in.


Cheers & hth.,

- Alf

 

[Leandro Melo]

I have an application where I have visual elements which are a: Moving
or Stationary, and b: Static or Animated

//
using namespace std;

//
class element
{
public:
     virtual void Paint (HDC hDC);

};

//
class movingElement : public element
{
public:
     void Move ();

};

//
class animatedElement : public element
{
public:
     void Update ();    // get next image in
                        // animated sequence

};

//
class movingAnimatedElement : public movingElement,
                               public animatedElement
{

};

So far, so good. Now I need a container to display all visual elements
sorted by e.g. Z-Order

//
class elementList
{
protected:
     list<element*>   m_list;

public:
     virtual void addElement (element* pEl);

};

Now, when adding all elements into a sorted list

//
void fn ()
{
     element     el;
     movingElement   mEl;
     animatedElement aEl;
     movingAnimatedElement maEl;

     elementList elList;

     elList.addElement (&el);    // ok
     elList.addElement (&mEl);   // ok
     elList.addElement (&aEl);   // ok
     elList.addElement (&maEl);
     Error C2594: 'argument' : ambiguous conversions from 'class
movingAnimatedElement*' to 'class element*'

}

C2594 is defined as "'operator' : ambiguous conversions from 'type1' to
'type1' No conversion from one specified type to the other was more
direct than any other. It may be necessary to define or specify an
explicit conversion."

If I convert maEl to a movingElement, I can't update the animated image
frame; if I convert it to an animatedElement, I can't move it. And I'd
rather not add "virtual void Move ()" to the definition of element and
everything derived from it. Any help, anyone?

Check the following FAQ items:

* http://www.parashift.com/c++-faq-lite/multiple-inheritance.html#faq-25.8
* http://www.parashift.com/c++-faq-lite/multiple-inheritance.html#faq-25.9

 

[Tonni Tielens]

I have an application where I have visual elements which are a: Moving
or Stationary, and b: Static or Animated

Like Alf already said: you probably have a design problem. Search
Google for "favor object composition over class inheritance".

 

[James Kanze]

I have an application where I have visual elements which are
a: Moving or Stationary, and b: Static or Animated

This sounds like you need the mixin pattern.

//
using namespace std;

//
class element
{
public:
virtual void Paint (HDC hDC);

Just curious, but is it possible that this should be pure
virtual. (Usually, the base class in such patterns only
contains pure virtual functions, but I suppose that there can be
exceptions.)

};

//
class movingElement : public element
{
public:
void Move ();
};

The question here is whether movingElement is a definitive type,
or just a mixin, implementing the "moving" functionality. If
the latter, it should be:

class movingElement : public virtual element
{
} ;

Also, it introduces a new public function. Is it supposed to be
an extension to the interface, or not. (I don't like mixing
partial implementations and extensions to the interface, but
there are times it's justified.)

If you're really thinking in terms of mixins, I like
implementing both alternatives, i.e. also defining a
stationaryElement, even if it is more or less empty. (This, of
course, is related to the concept of extending the interface or
not---a mixin shouldn't normally extend the interface.)

//
class animatedElement : public element
{
public:
void Update (); // get next image in
// animated sequence
};

Same comments as for movingElement.

//
class movingAnimatedElement : public movingElement,
public animatedElement
{
};

One of the reasons I like the idea of having a class for both
alternatives in the mixin strategy is that this can be cleanly
made into a template:

template< typename Movement, typename Animation >
class ConcreteElement
: public Element, private Movement, private Animation
{
// ...
} ;

Again, it's important to understand what you want to inherit,
interface or implementation. I'd avoid using the same
inheritance for both.

This type of solution does introduce a number of additional
classes. I find it cleaner because it separates the concerns,
but YMMV. If I wanted to avoid the extra classes, and rest
close to what you have done, all of the final classes should
inherit virtually from element, i.e.:

class movingElement : public virtual element
{
// ...
} ;

class animatedElement : public virtual element
{
// ...
} ;

class movingAnimatedElement
: public virtual element
, private /* ? */ movingElement
, private /* ? */ animatedElement
{
} ;

(The choice of private or public for movingElement and
animatedElement here depends on whether movingElement and
animatedElement are considered extensions to the interface or
not. Interfaces should be inherited publicly, implementations
privately.)

So far, so good. Now I need a container to display all visual
elements sorted by e.g. Z-Order

//
class elementList
{
protected:
list<element*> m_list;

Just a nit, but std::vector< element* > is probably a better
choice. Unless you want to go directly to std::set< element*,
ZOrderCmp >.

public:
virtual void addElement (element* pEl);
};

Now, when adding all elements into a sorted list

//
void fn ()
{
element el;
movingElement mEl;
animatedElement aEl;
movingAnimatedElement maEl;

elementList elList;

elList.addElement (&el); // ok
elList.addElement (&mEl); // ok
elList.addElement (&aEl); // ok
elList.addElement (&maEl);
Error C2594: 'argument' : ambiguous conversions from 'class
movingAnimatedElement*' to 'class element*'

C2594 is defined as "'operator' : ambiguous conversions from
'type1' to 'type1' No conversion from one specified type to
the other was more direct than any other. It may be necessary
to define or specify an explicit conversion."

That's because without the virtual inheritance, you don't have a
diamond inheritance, you have two instances of element in the
object, and it's ambiguous which one's address is wanted.

If I convert maEl to a movingElement, I can't update the
animated image frame; if I convert it to an animatedElement, I
can't move it. And I'd rather not add "virtual void Move ()"
to the definition of element and everything derived from it.
Any help, anyone?

I think the design needs a little bit more thought, with regards
to whether move and update should be part of the base interface,
or form extension to the interface; in the latter case (but that
may be just me), I'd separate the interface from the
implementation. But globally, the only real problem is the lack
of virtual in the inheritance.

 

[James Kanze]

* (e-mail address removed):

Well, no. You have two distinct base class objects of type
'element', one belonging to movingElement and on to
animatedElement. So if you try to call 'Paint' on a
movingAnimatedElement the compiler will report an ambigious
call: did you mean to call the movingElement Paint or the
animatedElement Paint?

Actually, the only Paint I see is element:aint. The question
is whether to call movingElement::element:aint or
animatedElement::element:aint. The same function, but on a
different object. (I'm also willing to bet that in the actual
code, Paint is overridden in the most derived class.)

And, noting that it doesn't make much sense to call *both*
(which could be arranged in the final bottom level class, but
isn't practically meaningful), you have a design level
problem, not just a C++ implementation problem.

Bullshit. About the only "problem" with the design is that
there doesn't seem to be a clear separation of implementation
and interface, and while I tend to insist on that, I'm not sure
that it's universally recognized as essential.

Adding "virtual" in the inheritance chain is /not/ a solution
of that design level problem -- for with 'element' a virtual
base class you still have the Paint problem.

What Paint problem?

Thus, the resolution hinges on what functionality you really
have in movingElement and in animatedElement.

I.e., does it really make sense to combine these two via
multiple inheritance (I think not, but could be, depending on
what they really are).

It probably does in some way. Whether he's found the optimal
way or not depends on factors we don't really know.

[...]

It's not a C++ problem, the solution is not "virtual"
inheritance,

There is a concrete, C++ problem, to which the solution is
virtual inheritance.

it is a design level problem, where at first I think it will
be helpful for you to focus on exactly what a Paint call on a
movingAnimatedElement should result in.

He mentionned "diamond inheritance" in the subject line, which
makes it fairly obvious that he only wants a single instance of
element in the object. And Paint() should be called on that
instance. The open design questions are more along the lines
how he wants to apply Move() or Update() when all he has are
element*, and whether Move() and Update() can really only have
one possible implementation (i.e. whether they should be virtual
or not).

 

[Alf P. Steinbach]

* James Kanze:

Actually, the only Paint I see is element:aint. The question
is whether to call movingElement::element:aint or
animatedElement::element:aint. The same function, but on a
different object. (I'm also willing to bet that in the actual
code, Paint is overridden in the most derived class.)

Yes, at this point it seems that you have understood this correctly, provided
that by "same" you mean overrides of the same virtual function.

Bullshit. About the only "problem" with the design is that
there doesn't seem to be a clear separation of implementation
and interface, and while I tend to insist on that, I'm not sure
that it's universally recognized as essential.

At this point, however, it seems that you haven't understood anything.

That makes it difficult to help you.

It might be that actually trying to implement a common Paint for two different
classes, one of them providing animation, might help, but I'm not sure whether
lack of understanding of that is the problem.

What Paint problem?

See above.

Thus, the resolution hinges on what functionality you really
have in movingElement and in animatedElement.

I.e., does it really make sense to combine these two via
multiple inheritance (I think not, but could be, depending on
what they really are).

It probably does in some way. Whether he's found the optimal
way or not depends on factors we don't really know.

[...]

It's not a C++ problem, the solution is not "virtual"
inheritance,

There is a concrete, C++ problem, to which the solution is
virtual inheritance.

There are more than one concrete C++ problem in the code presented. For example,
lack of implementation of a member function. And so on. It would just be silly
to point out any particular of those things as "the answer". Even if one of
those things is being actively fished for, with a few dangling odds and ends
(e.g. the "STL" in the article's title) so that one to a large degree suspects a
reformulated homework assignment. Instead, the thing fished for should IMHO be
pointed out is most probably /not/ a solution to the problem as presented here.


Cheers & hth.,

- Alf

 

[HGallon]

I have an application where I have visual elements which are a: Moving
or Stationary, and b: Static or Animated

<snip>

Dear All

Thanks for everyone's help. It is now clear to me that my design, rather
than any facet of virtual inheritance was at fault. A lot of code was
omitted from the sample code I provided, which may have hidden vital
details.

For what it is worth, my solution involved separating the image from the
element classes, and virtually inheriting a GetImage () function. I have
also separated the movement rules into a separate class so that I now have

class movingElement : public element, public movementRules
{
};

class animatedElement : public element
{
};

class movingAnimatedElement : public animatedElement, public movementRules
{
};

Instead of adding elements to the sorted list, I could then use:

class imageList
{
private:
std::list<image*> m_list; // all right, std::vector if you want

public:
void addImage (element& el);
};

void imageList::addImage (element& el)
{
image& img = element.getImage ();
// sort and list insertion code omitted
}