OOPsie: The Good, The Bad, The OOgly

Gamasutra reader Dustin Bennett asked the following in response to OOPsie: Individuality with Inheritance.  He suggested:

You should do a post about this line:
"Even though I mentioned this was a bad thing (creating all these inherited classes for a single property change)" and what we should do instead.

So here I am once again, to show you some of the pitfalls when it comes to OOP and how to avoid them.

Over-Inheritance

A simple pitfall many programmers fall into is over-inheritance, or overly defined class structures.  Let's look at an example using walls in Call of Duty: Modern Warfare 2.

Call of Duty: Modern Warfare 2

Here's some sample code to define the different walls we see above:

public class Wall
{
    protected String wallTexture;

    public Wall()
    {
        wallTexture = "defaultWallTexture";
    }

    public void Shine()
    {
        Console.WriteLine("Shining...");
    }
}

public class BrickWall : Wall
{
    public BrickWall()
        : base()
    {
        wallTexture = "brickWallTexture";
    }
}

public class DirtyBrickWall : BrickWall
{
    public DirtyBrickWall()
        : base()
    {
        wallTexture = "dirtyBrickWallTexture";
    }
}

public class BrickWallForLevel2Building1 : DirtyBrickWall
{
    public BrickWallForLevel2Building1()
        : base()
    {
        wallTexture = "lvl2-dirtyBrickTexture";
    }
}

public class ModernWarfareGame
{
    static void Main(string[] args)
    {
        Wall lvl2Wall = new BrickWallForLevel2Building1();
        lvl2Wall.Shine();
    }
}

As you can see, the developer started out with the best of intentions to create reusable code but took it a step or two too far and went to a level of code design that works against the reusability of objects that you strive for with OOP.  The programmer developed a wall class and then several inherited classes.  The parent, Wall, in this case is fine, and even BrickWall I have no problem with (forgetting the fact that we're only overridding to change a variable). 

But the question of appropriate design becomes to come into question when we hit the DirtyBrickWall class.  If we plan on using this wall all over the game, I'm fine with it.  However, if we only use it in one spot, there is probably some way to refactor the code and make a more reusable object.  This is even more apparent when we see the lowest level subclass, BrickWallForLevel2Building1.  Ignoring the fact that the class is named awfully, we shouldn't have objects that specific.  In most cases, we'll only use this object once and since there really isn't any differing functionality (only a variable difference) we should refactor this.  Here is the refactored code:

public class Wall
{
    protected String wallTexture;

    public Wall(String textureToUse)
    {
        wallTexture = textureToUse;
    }

    public void Shine()
    {
        Console.WriteLine("Shining...");
    }
}

public class ModernWarfareGame
{
    static void Main(string[] args)
    {
        Wall lvl2Wall = new Wall("lvl2-dirtyBrickTexture");
        lvl2Wall.Shine();
    }
}

Wow, that is so much clearer!  By providing a single argument in our constructor for wall, we essentially removed 4+ classes.  Usually this problem can be fixed by looking at the base object and analyzing the differences between the base and the subclasses as well as if those differences can be made generic and changing the base class.

It seems really basic that you shouldn't inherit if you don't plan on changing functionality, but sometimes we get blinded by the awesomeness of OOP and take it a bit overboard.  Often this leads to more problems down the road, typically with polymorphism.  Usually when I'm programming I strive for 2-3 'levels' of inheritance at max.  Any deeper than that and it begins getting a little specific.  With more detailed projects this number might increase by one or two, but it should never increase because of classes that don't change functionality of the base.

Now of course, you might have objects you only create one of, but the fact should remain that the class should be generic enough that you have the option to creating another object of that type (this is all assuming the class is not a Singleton).

Confusing Relationships

During a good portion of the OOPsie series we focused on the is-a relationship, defining inheritance as providing a subclass with an is-a relationship with the parent (for example, a Ferrari is-a car).  What we haven't looked at as much, the has-a relationship.  The has-a relationship is the relationship between a class and it's members, more specifically it's variables/properties.

A simple example of the has-a relationship is your age or your name.  Both are properties of you but you don't really inherit them from anything.  If we did, everyone would act the same if they were named the same or if they were the same age.  Now let's look at an incorrect code example showcasing the age relationship:

public interface Age
{
    int playerAge;
}

public class Player : Age
{
    public Player(int age)
    {
        playerAge = age;
    }
}

The above is properly written but poorly designed.  In this case, we have an interface called Age that the Player implements in order to gain access to the playerAge variable.  If we read this code, we could say 'player is-an age' which makes no sense!  It should be, 'player has-an age'.  We can fix this by simply removing the interface and making playerAge part of the Player class:

public class Player
{
    private int playerAge;

    public Player(int age)
    {
        playerAge = age;
    }
}

Simply remember, is-a relationships define inheritance while has-a relationships define class members (typically variables).

Quick side note: It's easy to see that defining incorrect relationships is another form of over-inheritance.

Conclusion

This article was just a quick diversion into some common OOP design problems rather than actual code errors.  Most problems many programmers have with OOP simply boil down to over-designing their class structures.  Sure, the final code may work, it just won't be an example of the elegance OOP strives for.  Continue to remember, always use OOP, but don't overly implement OOP.