Reducing code bloat: Unnecessary overloads example


Understanding how your algorithm works and its use cases is very important.

It allows you strip code you will never use (or better yet, never code for it). Or may be this could be seen as a case of overengineering.

Such is the example of Ogre’s LodStrategy.

There are multiple Lod strategies in Ogre, but they fall in two main categories:

  1. Distance based
  2. Pixel coverage based

The problem with these two categories is that one requires distance values to be sorted in ascending order: 0; 100; 200; 500; 1000; while the pixel values needs to be sorted in descending order: 5000px; 4000px; 2500px; etc

At the same time, the distance-based strategies make a distinction between “user values” and “internal values”. Basically, the user value is the actual distance, and the internal value is its squared.

Which by the way, that was an erronous optimization removed in 2.x because distance – radius <  lodThreashold is not the same as distance^2 – radius^2 <  lodThreashold^2

Going back to topic, the uglyness is that LodStrategy provides methods to sort & retrieve values in both ascending and descending order, and then uses a pure virtual for other classes to overload and select the correct version.

The class ends up looking like the following:

virtual Real transformUserValue(Real userValue) const; //Squares the result in distance lod.
virtual ushort getIndex( ... ) = 0;
virtual void sort() = 0;
virtual void isSorted() = 0;

protected:
static bool isSortedAscending( ... );
static bool isSortedDescending( ... );
static void sortAscending( ... );
static void sortDescending( ... );
static ushort getIndexAscending( ... );
static ushort getIndexDescending( ... );

This is awful. Everything useful ends up overloaded. This may look like intended OOP code but it’s hard to see which version ends up getting used by each method unless you have a clear understanding of the whole. Not to mention the virtuals increase code size and slow down the performance.
 

Optimizing the virtuals out

Did you notice something? we can transform user values into internal values… but somehow we can’t deal with its order???

Isn’t a > b the same as -a < -b ?

And that’s what we did!

Having two types of order is useless when you can negate the values that will be compared and kept in a list. All we need to do is keep the internal representation as negatives for Pixel strategies, and positive for Distance strategies.

The resulting code now looks more like this:

virtual Real transformUserValue(Real userValue) const; //Negates in pixel based, remains as is in distance based.
static ushort getIndex( ... );
static void sort( ... );
static void isSorted( ... );

 
Less virtuals, less code to maintain, cleaner. In short: less bloat.
Remember: If everything is virtual, it smells fishy. Try to use clever tricks and mathematical relationships to find a way to remove them.