2009-11-26

Multiple Inheritance and the Dreaded Diamond

Woopsi refactoring continues apace. Today’s latest changes were an attempt to consolidate the ListBox, ContextMenu and CycleButton gadgets.

Each of these gadgets is a different kind of view onto a list of items. The ContextMenu is the most primitive - it shows all of the items in its list as a vertical list. The ListBox is basically the same thing, except it introduces scrolling into the mix. The CycleButton shows just one option at a time; the other options can be paged through by clicking the button.

At present, the ListBox has the most advanced data mechanism. It leaves all of the data functionality to the ListData class, which wraps around a WoopsiArray (ie. vector) and provides such handy functions as sorting and item selection/deselection (enforcing single-only or multiple selection rules). It raises events to indicate changes to the data or selections within the data.

Meanwhile, the CycleButton and ContextMenu classes have very primitive data manipulation functionality. They both include an instance of the WoopsiArray and work with it directly. Wouldn’t it be better if they could benefit from the wealth of functionality in the ListData class?

Since the ContextMenu and ListBox work in very similar ways - the ContextMenu is basically a customised ListBox without scrolling - it makes sense to create a base class providing the most basic list view functionality that they can both inherit from. I split up the ListBox into “BasicListBox” (no scrolling) and “ListBox” (inherits from BasicListBox and ScrollingPanel, which adds the scrolling functionality). Here the problems began.

The BasicListBox inherits from the fundamental Gadget class, from which all Woopsi UI components inherit. The ScrollingPanel also inherits from the Gadget class. When the ListBox attempts to inherit from them both, it ends up with two copies of the Gadget class floating around in its inheritance hierarchy. Any attempts at working with the features of the Gadget class are now ambiguous and the compiler throws errors all over the place. This is called a “diamond” inheritance pattern:

           Gadget
       /             \
      /               \
     /                 \
BasicListBox    ScrollingPanel
     \                 /
      \               /
       \            /
           ListBox

The C++ FAQ Lite has a whole page about the diamond inheritance problem. The solution is to declare the inheritance from Gadget as virtual, which eliminates the ambiguities by eliminating the second copy of Gadget. However, attempting to do this has horrible problems in the Woopsi codebase. C-style casting cannot be used (uh oh) and the class at the top of the diamond (in this case, Gadget) should not include data members that need to be initialised (it can, but it shouldn’t).

The upshot of all this is that it is impossible to create a gadget that inherits from two other gadgets in Woopsi. If you think about it, this does make sense. It doesn’t help my goal of rationalising the list display gadgets, though. I could include an instance of the ScrollingPanel inside the ListBox to achieve the same effect, but that would be less efficient than the current solution. I could alternatively make the ListBox the basic list and have the ContextMenu inherit from that, and just not use the scrolling feature (though it would be handy if there are too many items to fit on-screen at once), but again that’s less efficient than the current solution.

I kept some of the changes I made before I reverted everything else back. ListData::swapItems() no longer raises a data changed event, as the only place it is called is in the sort() method. That method now raises the event instead. The ListDataItem struct has been replaced with a ListDataItem class, and it includes a C#/Java-style compareTo() method. Subclassing the ListDataItem and overriding this method allows the ListData class’ sort() method to sort the list differently. The ListBox’s scrolling canvas now has the correct height and the extra 1px at the bottom of every ListBox has now gone.

Finally, there are a couple of fixes in other places. The Woopsi class’ constructor no longer tries to retrieve the system font before it has been initialised, and the ContextMenu does not cast away the const-ness of the ContextMenuItem when raising a selection event to its listeners.