Thank God for Simple Code

As a programmer, I love learning new technologies and design patterns. I love dissecting code until I understand the minutia of how each piece works. I love reading blogs and StackOverflow posts about random technical topics, even if they’re not immediately applicable to anything I’m working on. And more often than not, when I go to lunch with other programmers, the discussion consists of weighing the pros and cons of different architectures for the features we’re working on.

Based on my experience, this thirst for technical knowledge is common among programmers. We find things cool that no ordinary person would, like using magic numbers to get around the high cost of computing the inverse square root. Code like this is celebrated among programmers. “How did they think of that? What a clever solution!”, we say.

That’s all well and good when the code is in isolation, but in reality, I would consider this bad code. It’s hard to understand, and therefore hard to maintain and debug. Don’t get me wrong, every application has that 10% of code which is executed millions of times per second and needs to be heavily optimized, but most code does not fall into that category. It might be fun to save a few nanoseconds by doing some crazy optimization, or to use that design pattern you just read about, or to include that new open source library that was just released, but most of the time it’s not worth the complexity.

Instead, when you’re writing code as part of a team, readability and maintainability should be your primary focus. As Steve McConnell says in his popular programming book Code Complete: “Software’s Primary Technical Imperative has to be managing complexity”. I would actually take that a step further and say if you ever use a language feature, design pattern, or library for any reason other than to make the code simpler, you’re writing bad code. Why, you ask? Let’s look at a (contrived) example.

Contrived Example

Let’s say you’re the billionaire owner of the popular social networking site, CatEnthusiast.com. Since you have so much extra time and money (they always doubted you!), you ask two of your programmers, Alice and Bob, to build separate implementations for your company’s hot new feature: a cat-to-human-name converter. Users input their cat’s name and they get back their cat’s “human” name. The only requirement is that each cat name maps to a single human name, otherwise we’d have a lot of confused cats on our hands.

Alice approaches the feature by stripping it down to its simplest form: a string to string map. She just populates a list with names at initialization time and keeps track of which names are free and which are taken.

Here’s what her code would look like:

class SimpleCatNameConverter
{
   public:
      ...
      std::string GetHumanName(const std::string& catName);

   private:
      std::list<std::string> mUnclaimedHumanNames;
      std::map<std::string, std::string> mCatToHumanNamesMap;
}
std::string SimpleCatNameConverter::GetHumanName(const std::string& catName)
{
   // if the specified cat name already has a human name assigned 
   // to it, return it
   if (mCatToHumanNamesMap.find(catName) != mCatToHumanNamesMap.end())
   {
      return mCatToHumanNamesMap[catName];
   }

   // otherwise assign the first available name to the cat
   std::string humanName = mUnclaimedHumanNames.front();
   mUnclaimedHumanNames.pop_front();

   mCatToHumanNamesMap[catName] = humanName;

   return humanName;
}

Bam, done. Her mapping function is less than 20 lines of code, and uses extremely simple data structures that all programmers are familiar with (std::list and std::map). The entire feature took her an hour to write and test.

Meanwhile, Bob is thinking about all of the possible ways this feature can expand in the future. Maybe users won’t be happy with just receiving a completely random human name for their cat, maybe they’ll want to specify some criteria it has to meet (i.e. “I want a name that starts with an ‘S'”, “I want a name that rhymes with ‘Mavid'”, etc.).

Bob wants to write a robust, flexible system using the factory design pattern that can support all of these inevitable crazy cat naming schemes. First, he makes an abstract interface that all of his cat naming algorithms will implement:

class ICatNameConverter
{
   public:
      virtual std::string GetHumanName(const std::string& catName) = 0;
}

Alright, Bob’s implementation looks pretty similar to Alice’s so far, except Bob will also have to write at least two extra classes: a concrete implementation of ICatNameConverter, and the factory that decides which concrete class to make. His concrete implementation matches Alice’s code exactly, except it inherits from ICatNameConverter:

class SimpleCatNameConverter : public ICatNameConverter
{
   public:
      ...
      std::string GetHumanName(const std::string& catName) override;

   private:
      std::list<std::string> mUnclaimedHumanNames;
      std::map<std::string, std::string> mCatToHumanNamesMap;
}

And finally, his factory class which will decide how to convert the name based on the cat name type:

class CatNameConverterFactory
{
   public:
      enum CatNameConverterType
      {
         kCatNameConveterTypeSimple
         // TODO: Future cat name converters go here.
      };

      // Factory method used to get cat name converters. 
      static ICatNameConverter* GetCatNameConverter(CatNameConverterType type);
}

Phew, OK, now Bob’s done too. He took a bit longer than Alice, but the flexibility will totally be worth it!…Right? Anyway, you decide to deploy both implementations in parallel, with half of the world getting Alice’s implementation and the other half getting Bob’s.

And Then…Disaster!

Flash forward 6 months. The Cat Name Converter is a runaway success (you knew it would be!). Bob has added several new ICatNameConverter implementations, while Alice has left her SimpleCatNameConverter as-is since users seemed happy with it. Both Alice and Bob are off on well-deserved vacations in remote locations around the world and have no access to the internet or cell phones.

And, of course, this is when disaster strikes! Users start reporting that different cats are getting assigned the same human name. Oh, the humanity! You’re not sure if the bug is in Alice or Bob’s implementation, but you ask your newest programmer, Jimmy, to look into it and fix the issue.

Jimmy starts off by opening up Alice’s implementation and giving it a quick read through. Since the code is so dead-simple, he immediately understands the algorithm and intended behavior. He runs some test cases on her code using cat name inputs, and everything works as expected. After running through some edge cases in his head, Jimmy is satisfied that the bug must be in Bob’s implementation, not Alice’s.

The next step, obviously, is for Jimmy to open up Bob’s code and read it. But where should he start? CatNameConverterFactory looks as good a place as any. Unlike Alice’s code, it features a few extra cat name converter types:

class CatNameConverterFactory
{
   public:
      enum CatNameConverterType
      {
         kCatNameConveterTypeSimple,
         kCatNameConveterTypeFirstLetter,
         kCatNameConveterTypeRhymesWith
      };

      // Factory method used to get cat name converters. 
      static ICatNameConverter* GetCatNameConverter(CatNameConverterType type);
}

So Bob has three cat name converter classes instead of one, not a big deal. Jimmy reads through them, thinking of edge cases and running tests, just like he did with Alice’s code. It takes him longer because he has to keep a lot more information in his head at once, but the implementations look solid and his unit tests all pass.

However, Jimmy finally reproduces the bug when he runs Bob’s code with some real-world input. It turns out that the bug only manifests with some specific combination of different name converter types. And to make matters worse, it the bug isn’t 100% reproducible. Jimmy sees some calls to rand() in Bob’s kCatNameConveterTypeRhymesWith implementation, which might explain that.

A week has passed since cats started receiving duplicate names, and they’re starting to grow restless. Jimmy starts to panic. He runs test after test, trying to isolate the exact combination of cat name converters that triggers the bug. He’s getting closer, but the code is so abstract and indirect that he can’t build a good mental model of it all. Things were going so smooth for Jimmy, but now he doesn’t know if he’ll keep his job, let alone get that raise he was hoping for. Why, Bob, why!? Why couldn’t you just keep things simple?!

Finally, after dozens of hours of debugging, Jimmy finds and fixes the bug. He might have a few more grey hairs, but all is finally as it should be. Unfortunately, some irreparable damage has been done to CatEnthusiast.com, with millions of angry users closing their accounts and moving to your competitor, FelineFan.net. Bob’s going to have some real explaining to do when he gets back!

Think Simple, Code Simple, Live Simple

Obviously this example was an exaggerated worst-case scenario, but it’s representative of real experiences I’ve personally had or witnessed many times. Unlike code written for personal toy projects, code written for real-world applications is much more complex and is shared by a large team of diverse programmers. They spend as much time reading and debugging code as writing new code. As such, it’s of the utmost importance that you focus on simplicity and readability when writing code as part of a team (or even by yourself; you’ll probably come back to your own code in a few months, too). It will save you, your colleagues, and your company lots of headaches and sorrow.

Programmers sometimes equate “simple” code to “bad” code or “stupid” code. On the contrary, I would say that writing a simple, easy to grasp implementation for a feature is harder than writing a complex, intricate implementation. I know I appreciate whenever I open up some code to debug and I’m greeted with a clean, well-commented implementation that is relatively short.

Instead of adding unnecessary complexity up front, try to make your code as simple as possible. Only use language features (i.e. abstract methods, inheritance, lambdas, templates, etc.), design patterns, and libraries when they simplify your code. Best case, you never touch the code again and it’s so simple that there are obviously no bugs in it. Worst case, the requirements for the feature become so complicated that you have to throw away a small amount of code and rewrite the system. Either way, it’s better than writing a confusing system that other coders hate to maintain.

So, the next time you run across some really simple code that is easy to understand, take a moment to celebrate it.

Advertisements

6 thoughts on “Thank God for Simple Code

  1. Just to point out, Bob’s implementation did more things. If users were using his code, it offered more value to the users and the sites. The implementations differed in complexity because of the difference in feature sets.

    • I agree that Bob’s implementation had more logic than Alice’s, but I’d argue that it doesn’t justify the extra complexity. Bob only has three cat naming algorithms. In my mind, having three “types” of anything doesn’t justify the extra layer of abstraction that the factory pattern adds. Alice could have easily matched Bob’s behavior by taking a CatNameConverterType parameter, switching off of it, and having three helper methods that each implement one algorithm type.

      Of course, each approach has its pros and cons. There are definitely situations where a factory pattern is justified, but that should be decided on a case by case basis. In this case, it would depend on how complex Bob’s extra naming algorithms are, as well as how often users are requesting new algorithms. If the algorithms are all relatively straightforward and users have been happy with three algorithms for months, then I’d argue it’s better to skip the factory. If not, then go for it!

  2. Debugging is twice as hard as writing the code in the first place. So if you write code as complex as you possibly can you by definition are not smart enough to debug it.

    Learn that and live by that.

  3. “if you ever use a language feature, design pattern, or library for any reason other than to make the code simpler, you’re writing bad code.”

    Efficiency is sometimes a perfectly valid reason. I sometimes use Google’s sparsehash library. Not because it’s simpler than just using standard library containers, but because it provides the performance characteristics I need.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out /  Change )

Google+ photo

You are commenting using your Google+ account. Log Out /  Change )

Twitter picture

You are commenting using your Twitter account. Log Out /  Change )

Facebook photo

You are commenting using your Facebook account. Log Out /  Change )

Connecting to %s