Writing Sane JavaScript Functions

Meet Reggie

Reggie writes JavaScript all day. He sits in his cubical, has his VSCode editor opened up all the time and likes to write big, long, ugly functions with his VSCode editor. Let’s take a look at one of Reggie’s famed functions.

const params = {
  tool: true,
  multi: 'some string',
}

async function updateFlurb (data) {
  const res = await request.post('http://www.example.com', data)
  return res.body
}

async function doItAll (data, opts, useDefaults) {
  const newDate = data.oldDate + 1

  opts.hello = "Reggie mutating an input arg"

  if (params.tool) {
    if (useDefaults && data.useParams) {
      data.hello = `${params.multi} is like ${data.hello} the only ${opts.bar}`
    }
  }

  const res = updateFlurb(data.stuff)

  return {
    wat: data.wat,
    res: res.boing,
    data,
    newDate,
  }
}

Reggie’s done it again, he’s made another really messy function that is not only hard to test, hard to debug, but hard to understand what the dotItAll function really does. Let’s take a look at what Reggie is doing so wrong.

Only do one thing

Reggies’s doItAll function tries to do too much. The challenge, make more functions that only do one thing. We see Reggie’s function mutating variables, fetching data from a remote URL, transforming the data object. It’s a totally mess. Instead all these different functions should be split out. Do one thing. That’s OK if you have a bunch of two line functions. Having individual functions allows you to unit test so much easier and the functions can be used by other functions. That’s what we’re doing, making functions that can be combined like Lego pieces.

Set sane defaults

Reggie assumes that args data, and opts, and even useDefaults are filled in. This is a bad assumption to make. Most of the time, this function has one client/caller and we know the client/caller already checks for this value and provides it. This is known as Guilty Knowledge.

Guilty knowledge: Knowledge the function has, but it really shouldn’t because it’s not its job to know.

Your function should know nothing outside of what it’s passed and the lines within it. It should be a black box, not caring about the outside world, only concerned with what comes through the front door and what leaves out the backdoor.

Never Trust your inputs

You see where Reggie doesn’t even test if data is an object, but tries to grab a property off it using data.oldDate? This is Reggie trusting his inputs, do not trust them. It doesn’t matter if you know the only thing calling function has already checked. That’s more Guilty Knowledge.

Do not mutate the arguments

Did you see that? He did it again. opts.hello = "Reggie mutating an input arg". This mutates the object. The caller might have already set opts.hello, but here we plow over it like some drunken Canadian on a Zamboni. Instead create a clone of the object and return the cloned object with the new value. Mutating args makes debugging more difficult to track down to the error.

Avoid nested If’s like the plague

What’s so bad about nested ifs? Too much prone to error. They may seem innocent, but for a new person stumbling onto your code, it will take cognitive discipline to decipher the original intent. Don’t do it. Pass that stuff out into another function.

Give feed back on failures

Poor Reggie doesn’t catch errors, let alone provide feedback. At the beginning of your function, check the sanity of your arguments and error out with some nice feedback. For example.

async function doItAll (data, opts, useDefaults) {
  if (!data) throw new Error('No data provided')
}

By throwing a real Error (not just returning undefined, false, or throwing a String, you allow the caller to decide how to handle the error. Retry? Fail to the customer, try another method.

Be flexible in your input and strict on your output

I prefer to use objects and destructure them when I a function has multiple arguments.

function getBingBangBoo ({ id, store = true, people = [] }) {
...
}

You can see here, that I use destructuring to be flexible in the order of the arguments passed and if some arguments are passed at all.

KISS

This harkens back to do one thing. Don’t be afraid to have lots of functions that just have one or two lines. So much easier to test and reuse. The problem you’ll encounter with more functions is naming them, but that’s manageable.

Keep things simple in your functions. You have no need for fancy patterns that take time to figure out what is actually happening. I would rather have three clear, simple lines of code than one complex obscure line of code that does the same thing. Simple, your future self will thank you.

Conclusion

That’s it. write / test lots of functions. Keep them simple and you’ll have code that’s easy to test, refactor, and grok.

References

Some really good ideas when building software

• YAGNI: You Aren’t Gonna Need It. Chances are, that fancy new thing you are adding to your project, isn’t really needed.
• KISS: Keep it Stupid Simple. Simple > (Clever OR Performant OR Faster OR Shorter OR Smaller). Simple always wins.
• Worse is Better: Software that is limited, but simple to use, may be more appealing to the user. See, simple is better
• SOLID: Five design principles that make software more understandable.