Keep your code dumb
Tags: javascript, codesmells
Published: 2017-11-15 21:43:12

---
title: Keep your code dumb
published: true
description: 
tags: javascript, codesmells
---

I recently stumbled across this tweet...

{% twitter 929851552172519424%}

The proposed solution was to use a getter to populate the value at the time you need it, something similar to this:

```javascript
function getUniqueId() {
  // pretend this is some expensive computation
  return '--id--'
}

function MyObject () {
}

// lazy getter for 'id'
Object.defineProperty(MyObject.prototype, 'id', {
  get: function() {
    const value = getUniqueId()
    Object.defineProperty(this, 'id', { value, enumerable: true })
    return value
  }
})

const obj = new MyObject()
console.log(obj) // > {}
console.log(obj.id) // > "--id--"
console.log(obj) // > { id: "--id--" }
```

Upon initial glance, this code looks _very clever_. It may work well today, but bugs with code like this will be difficult to track down later or even worse catastrophic to a codebase.

I maintain a large legacy C# project that currently has lazy getters. This code was written many years ago by the ghosts of programmers past and at the time it was written was very clever.

Something similar to this:

```c#
// BAD: clever code
public class House
{
  private _cats Cat[];

  public int Id { get; set; }

  // Lazy property that "does work". "work" should be in a function, not prop.
  public Cat[] Cats
  {
    get
    {
       if (_cats == null)
       {
         _cats = db.GetCats(Id);
       }

       return _cats;
    }
  }
}

```

Today this clever code is biting us in the ass.

There was a recent project to use Redis for caching. It was realized (after launching to production) that every lazy value is now enumerated during the serialization process for caching. It ended up causing such a massive spike in CPU on production that the deployment had to be rolled back.

Because multiple things were in that deployment, it took the teams a while to determine it was the new Redis code that was causing the CPU spike and even longer time to determine _why_ the Redis code was causing a CPU spike.

Had we used dumber code like ...

```C#
// GOOD: dumb code
public class House
{
  public Cat[] GetCats()
  {
    // ...
  }
}

```

... we would never have run into this problem.

The codebase is now over a million lines and the fix for a problem that should have never existed is monumental. It is still unfixed to this day.

This isn't the first time this same clever code has bit us in the ass. There are places in the code I will see things like this:

```C#
// BAD: In this code, `house.Cats` has to be touched
//      in order for the property to be populated.

var house = db.GetHouse(id);

// WTF?
house.Cats;

return DoSomething(house);
```


This type of code requires me to dig into the implementation of the property `id` to understand it's usage. This code smell is so bad it makes me wretch.

**Too many times I have seen clever code come back and haunt the codebase.**

Clever code will leave you to hunt down and solve equally clever bugs. Keep your programs dumb and your code and your bugs will be easier to find.

I would love to hear stories on how have you been bitten by clever code in the comments below!

Follow me on Twitter [@joelnet](https://twitter.com/joelnet) or [LinkedIn](https://www.linkedin.com/in/joel-thoms/)

Cheers!