Problems with object-oriented programming

There’s a lot of writing out there about what “good code” looks like, especially within the object-oriented programming community. I mostly work with imperative and object-oriented programming languages, but a lot of the ideas I’ve heard about what “proper OOP” looks like don’t resonate with me.

I recently flipped through a copy of 99 Bottles of OOP, which is one such “what does good object-oriented code look like” book. I’ve heard good things about it, and I was curious how much it would align with my opinions and how much I’d disagree with. And, well… I disagreed with a lot.

The premise of the book is that it presents us with the problem of writing a program that prints out the “99 bottles of beer on the wall” song, and then walks us through an object-oriented development process to create a program using strategies that scale to larger projects. I’m not very impressed by the code the book comes up with, though.

Here’s the solution they provide:

class Bottles {
    song() {
        return this.verses(99, 0);
    }

    verses(starting, ending) {
        return downTo(starting, ending)
            .map(i => this.verse(i))
            .join('\n');
    }

    verse(number) {
        const bottleNumber = BottleNumber.for(number);

        return (
            capitalize(`${bottleNumber} of beer on the wall, `) +
            `${bottleNumber} of beer.\n` +
            `${bottleNumber.action()}, ` +
            `${bottleNumber.successor()} of beer on the wall.\n`
        );
    }
}

class BottleNumber {
    static for(number) {
        let bottleNumberClass;
        switch (number) {
            case 0:
                bottleNumberClass = BottleNumber0;
                break;
            case 1:
                bottleNumberClass = BottleNumber1;
                break;
            case 6:
                bottleNumberClass = BottleNumber6;
                break;
            default:
                bottleNumberClass = BottleNumber;
                break;
        }

        return new bottleNumberClass(number);
    }

    constructor(number) {
        this.number = number;
    }

    toString() {
        return `${this.quantity()}${this.container()}`;
    }

    quantity() {
        returnthis.number.toString();
    }

    container() {
        return 'bottles';
    }

    action() {
        return `Take ${this.pronoun()} down and pass it around`;
    }

    pronoun() {
        return 'one';
    }

    successor() {
        return BottleNumber.for(this.number - 1);
    }
}

class BottleNumber0 extends BottleNumber {
    quantity() {
        return 'no more';
    }

    action() {
        return 'Go to the store and buy some more';
    }

    successor() {
        return BottleNumber.for(99);
    }
}

class BottleNumber1 extends BottleNumber {
    container() {
        return 'bottle';
    }

    pronoun() {
        return 'it';
    }
}

class BottleNumber6 extends BottleNumber {
    quantity() {
        return '1';
    }

    container() {
        return 'six-pack';
    }
}

The book is very proud of this code. It describes it like this —

This code is a tribute to the intense simplicity achieved by deft handling of complexity. It’s a testament to the efficacy of programming by identifying smells and removing them using well-known refactoring recipes.

— and that seems awfully generous to me.

Suppose you want to find out what verse the code prints when there are six bottles of beer on the wall.

• First you look at the song method, which tells you each verse is constructed with verse.
• You look at verse, which tells you that the verse number is turned into a BottleNumber through the constructor method for.
• We scroll down to BottleNumber.for and see that when there are six bottles of beer, we construct a BottleNumber6 object.
• We go back to verse and take a look at the rest of the method, and try to figure out what that prints.
• In order to find out how ${bottleNumber} is interpreted, we scroll down to BottleNumber6 and check whether it has a custom toString method.
• It doesn’t, so we scroll back up to BottleNumber and read its toString method, which calls the quantity and container methods. So we scroll back down to BottleNumber6 to see if it has a custom quantity method, which it does, and whether it has a custom container, which it also does.
• Likewise, we scroll down to check whether BottleNumber6 has a custom action method. It does not, so we scroll back up to read the default action method, which invokes the pronoun method. Again, we scroll down to look for an overridden pronoun method on BottleNumber6, but we do not find one. So we scroll back up and read the default pronoun implementation.
• Lastly, we scroll down to BottleNumber6 to look for a successor implementation. There isn’t one, so we scroll back up and read the default successor method, which mercifully does not call any other methods.

I hope that list was less painful to read than it was to write. No wonder the code feels hard to read — everything is so indirected!

Here’s how I’d write this program:

function composeSong(numberBottles=99) {
    return inclusiveRange(0, numberBottles)
        .reverse()
        .map((i) => verse(i, numberBottles))
        .join("\n")
}

function verse(number, startNumber) {
    let phrases1 = findLinePhrases(number)
    let nextNumber = number == 0 ? startNumber : number - 1
    let phrases2 = findLinePhrases(nextNumber)
    let result =
        `${capitalize(phrases1.quantity)} ${phrases1.items} `
        + `of beer on the wall, `
        + `${phrases1.quantity} ${phrases1.items} of beer.\n`
        + `${capitalize(phrases1.action)}, `
        + `${phrases2.quantity} ${phrases2.items} `
        + `of beer on the wall.`
    return result
}

function findLinePhrases(number) {
    let quantity, items, pronoun, action

    if      (number == 0) { quantity = "no more" }
    else if (number == 6) { quantity = String(1) }
    else                  { quantity = String(number) }

    if      (number == 1) { items = "bottle" }
    else if (number == 6) { items = "six-pack" }
    else                  { items = "bottles" }

    if (number == 1) { pronoun = "it" }
    else             { pronoun = "one" }

    if (number == 0) { action = "go to the store, buy some more" }
    else             { action = `take ${pronoun} down, pass it around` }

    return {quantity: quantity, items: items, action: action}
}

That’s less than half as long as the original implementation, and substantially more legible. If you want to find out what the program prints when there are six bottles, you read printSong, which points you to verse. The template for the verse is in verse, and all of the phrases that fill in that template are determined in findLinePhrases. There’s no scrolling from parent class to subclass to trace behaviour. Nothing relevant is hidden away from you. All of the program logic is condensed into four brief conditionals, almost reminiscent of a small spreadsheet. This, I consider an example of intense simplicity.

I don’t think many people would dispute that this version is simpler than the original. An OOP advocate might argue, however, that it is less extensible and maintainable than the original polymorphic example. I disagree with that.

Instead of being thinly spread out over a variety of classes, the logic of the second program is all in one place. You always know where the code you’re looking for is without having to search for it. It’s easy to add a row to one of the conditionals, to change a row, or to add another conditional. Being concise is not the same as being hard to work with. In this case, I think being concise makes it easier to work with.

In my version of the code, I have findLinePhrases split into four conditionals. I could refactor it into one big conditional, if I wanted to. I have it split up this way because I think it’s nicer, but we’re at a point where we could really go either way. If the logic leans towards a few distinct alternatives, one big conditional might be the clearest option. In other cases, though, setting it up as one big conditional leads to real problems.

What if we want to do this?

if      (number == 0)            { quantity = "no more" }
else if (number == 6)            { quantity = String(1) }
else if (hasThreeDigits(number)) { quantity = `${number} (three digits!)` }
else                             { quantity = String(number) }

if      (number == 0)    { action = "go to the store, buy some more" }
else if (isEven(number)) { action = `take ${pronoun} down, put it in the fridge` }
else                     { action = `take ${pronoun} down, pass it around` }

This is harder to do if we have findLinePhrases implemented as one big conditional. As you add in overlapping conditions, the number of cases you have to write into that conditional grows rapidly. And when you implement your logic using a class hierarchy, you’re necessarily using one big conditional. You’re going to have to create new 3DigitBottleNumber and EvenBottleNumber classes. And what about the even numbers with three digits? Do they use multiple inheritance, or do we refactor from inheritance to composition & build our BottleNumbers with a BottleNumberBuilder?

I don’t think polymorphism is a good approach in this situation. In my opinion, the “bottles of beer” song is a pretty good example of a problem which invites a simple conditional solution.

Now, none of this is to say that class-based polymorphism doesn’t have its place. It absolutely does. For example, suppose we wanted a filesystem abstraction that could read and write files to a variety of storage backends, including local disks, remote disks, and cloud object storage. It absolutely makes sense to create a polymorphic interface which abstracts the file access methods across those backends. Each backend has wildly divergent behaviour and needs to be constructed in different ways, but we want them to be used interchangeably in a variety of contexts. Maybe the different backends are even being developed by different teams. Trying to centralize all those different implementations into one set of conditionals would be much more complicated than just splitting it up.

But 99 Bottles of OOP thinks that this kind of polymorphic solution should be used everywhere.

As an OO practitioner, when you see a conditional, the hairs on your neck should stand up. Its very presence ought to offend your sensibilities. You should feel entitled to send messages to objects, and look for a way to write code that allows you to do so.

I have a real problem with this. See, while polymorphism is an excellent choice for high-level, strongly-divergent logic like a storage backend, most of the code we write isn’t at that level. Day to day, most of the problems we deal with are bottles-of-beer problems. And when you apply high-level architectural patterns to pedestrian situations, you wind up with code that is more than twice as long as it needs to be, and substantially harder to read and work with.

I think the book’s perspective here is far too absolutist. We should prefer conditionals in most situations. I don’t have a problem with objects or classes; they’ve absolutely got their uses. But I do have a problem with a particular notion of “Good and Proper Object-Oriented Code” which asks us to use objects and classes everywhere, even when they’re the wrong tool for the job.

There’s a quote from Code Complete which goes like this:

Classes and routines are first and foremost intellectual tools for reducing complexity. If they’re not making your job simpler, they’re not doing their jobs.

I find myself reminded of that quote a lot when I engage with the 99 Bottles of OOP school of philosophy. When classes and objects are just getting in our way and making our code harder to understand and modify, we shouldn’t be using them.