Writing code that makes mistakes harder – Donny Wals


As we work on projects, we usually add more code than we remove. At least that’s how things are at the beginning of our project. While our project grows, the needs of the codebase change, and we start refactoring things. One thing that’s often quite hard to get exactly right when coding is the kinds of abstractions and design patterns we actually need. In this post, I would like to explore a mechanism that I like to leverage to make sure my code is robust without actually worrying too much about abstractions and design patterns in the first place.

We’ll start off by sketching a few scenarios in which you might find yourself wondering what to do. Or even worse, scenarios where you start noticing that some things go wrong sometimes, on some screens. After that, we’ll look at how we can leverage Swift’s type system and access control to prevent ourselves from writing code that’s prone to containing mistakes.

Common mistakes in codebases

When you look at codebases that have grown over time without applying the principles that I’d like to outline in this post, you’ll often see that the codebase contains code duplication, lots of if statements, some switch statements here and there, and a whole bunch of mutable values.

None of these are mistakes on their own, I would never, ever argue that the existence of an if statement, switch, or even code duplication is a mistake that should immediately be rectified.

What I am saying is that these are often symptoms of a codebase where it becomes easier and easier over time to make mistakes. There’s a big difference there. The code itself might not be the mistake; the code allows you as a developer to make mistakes more easily when it’s not structured and designed to prevent mistakes.

Let’s take a look at some examples of how mistakes can be made too easy through code.

Mistakes as a result of code duplication

For example, imagine having a SwiftUI view that looks as follows:

struct MyView {
  @ObservedObject var viewModel: MyViewModel

  var body: some View {
    Text("\(viewModel.user.givenName) \(viewModel.user.familyName) (\(viewModel.user.email))")
  }
}

On its own, this doesn’t look too bad. We just have a view, and a view model, and to present something to the user we grab a few view model properties and we format them nicely for our user.

Once the app that contains this view grows, we might need to grab the same data from a (different) view model, and format it identical to how it’s formatted in other views.

Initially some copying and pasting will cut it but at some point you’ll usually find that things get out of sync. One view presents data one way, and another view presents data in another way.

You could update this view and view model as follows to fix the potential for mistakes:

class MyViewModel: ObservableObject {
  // ...

  var formattedUsername: String {
    return "\(user.givenName) \(user.familyName) (\(user.email))"
  }
}
struct MyView {
  @ObservedObject var viewModel: MyViewModel

  var body: some View {
    Text(viewModel.formattedUsername)
  }
}

With this code in place, we can use this view model in multiple places and reuse the formatted name.

It would be even better if we moved the formatted name onto our User object:

extension User {
  // ...

  var formattedUsername: String {
    return "\(givenName) \(familyName) (\(email))"
  }
}

struct MyView {
  @ObservedObject var viewModel: MyViewModel

  var body: some View {
    Text(viewModel.user.formattedUsername)
  }
}

While this code allows us to easily get a formatted username wherever we have access to a user, we are violating a principle called the Law of Demeter. I have written about this before in a post where I talk about loose coupling so I won’t go too in depth for now but the key point to remember is that our view explicitly depends on MyViewModel which is fine. However, by accessing user.formattedUsername on this view model, our view also has an implicit dependency on User. And not just that, it also depends on view model having access to a user object.

I’d prefer to make one more change to this code and make it work as follows:

extension User {
  // ...

  var formattedUsername: String {
    return "\(givenName) \(familyName) (\(email))"
  }
}

class MyViewModel: ObservableObject {
  // ...

  var formattedUsername: String {
    return user.formattedUsername
  }
}
struct MyView {
  @ObservedObject var viewModel: MyViewModel

  var body: some View {
    Text(viewModel.formattedUsername)
  }
}

This might feel a little redundant at first but once you start paying attention to keeping your implicit dependencies in check and you try to only access properties on the object you depend on without chaining multiple accesses you’ll find that making changes to your code suddenly requires less work than it does when you have implicit dependencies all over the place.

Another form of code duplication can happen when you’re styling UI elements. For example, you might have written some code that styles a button in a particular way.

If there’s more than one place that should present this button, I could copy and paste it and things will be fine.

However, a few months later we might need to make the button labels bold instead of regular font weight and it will be way too easy to miss one or two buttons that we forgot about. We could do a full project search for Button but that would most likely yield way more results than just the buttons that we want to change. This makes it far too easy to overlook one or more buttons that we should be updating.

Duplicating code or logic once or twice usually isn’t a big deal. In fact, sometimes generalizing or placing the duplicated code somewhere is more tedious and complex than it’s worth. However, once you start to duplicate more and more, or when you’re duplicating things that are essential to keep in sync, you should consider making a small and lightweight abstraction or wrapper to prevent mistakes.

Preventing mistakes related to code duplication

Whenever you find yourself reaching for cmd+c on your keyboard, you should ask yourself whether you’re about to copy something that will need to be copied often. Since none of us have the ability to reliably predict the future, this will always be somewhat of a guess. As you gain more experience in the field you will develop a sense for when things are prone to duplication and a good candidate to abstract.

Especially when an abstraction can be added in a simple manner you shouldn’t have a very high tolerance for copying and pasting code.

Consider the view model example from earlier. We were able to resolve our problem by making sure that we thought about the right level of placing our user’s formatted name. Initially we put it on the view model, but then we changed this by giving the user itself a formatted name. Allowing any place that has access to our user object to grab a formatted name.

An added benefit here is we keep our view model as thin as possible, and we’ve made our user object more flexible.

In the case of a button that needs to appear in multiple places it makes sense to wrap the button in a custom view. It could also make sense to write a custom button style if that better fits your use case.

Mistakes as a result of complex state

Managing state is hard. I don’t trust anybody that would argue otherwise.

It’s not uncommon for code to slowly but surely turn into a complex state machine that uses a handful of boolean values and some strings to determine what the app’s current state really is. Often the result is that when once boolean is true, a couple of others must be false because the program would be in a bad state otherwise.

My favorite example of a situation where we have multiple bits of state along with some rules about when this state is or isn’t valid is URLSession‘s callback for a data task:

URLSession.shared.dataTask(with: url) { data, response, error in
  guard error == nil else {
    // something went wrong, handle error
    return
  }

  guard let data, let response else {
    // something went VERY wrong
    // we have no error, no data, and no response
    return
  }

  // use data and response
}

If our request fails and comes back as an error, we know that the response and data arguments must be nil and vice-versa. This is a simple example but I’ve seen much worse in code I’ve worked on. And the problem was never introduced knowingly. It’s always the result of slowly but surely growing the app and changing the requirements.

When we design our code, we can fix these kinds of problems before they occur. When you notice that you can express an impossible state in your app due to a growth in variables that are intended to interact together, consider leveraging enums to represent the states your app can be in.

That way, you significantly lower your chances of writing incorrect state into your app, which your users will enjoy.

For example, Apple could have improved their URLSession example with the Result type for callbacks. Luckily, with async / await bad state can’t be represented anymore because a data call now returns a non-optional Data and URLResponse or throws an Error.

Mistakes as a result of not knowing the magical incantation

One last example that I’d like to highlight is when codebases require you to call a series of methods in a particular order to make sure that everything works correctly, and all bookkeeping is performed correctly.

This is usually the result of API design that’s somewhat lacking in its usability.

One example of this is the API for adding and removing child view controllers in UIKit.

When you add a child view controller you write code that looks a little like this:

addChild(childViewController)
// ... some setup code ...
childViewController.didMove(toParent: self)

That doesn’t seem too bad, right.

The syntax for removing a child view controller looks as follows:

childViewController.willMove(toParent: nil)
// ... some setup code ...
childViewController.removeFromParent()

The difference here is whether we call willMove or didMove on our childViewController. Not calling these methods correctly can result in too few or too many view controller lifecycle events being sent to your child view controller. Personally, I always forget whether I need to call didMove or willMove when I work with child view controllers because I do it too infrequently to remember.

To fix this, the API design could be improved to automatically call the correct method when you make a call to addChild or removeFromParent.

In your own API design, you’ll want to look out for situations where your program only works correctly when you call the right methods in the right order. Especially when the method calls should always be grouped closely together.

That said, sometimes there is a good reason why an API was designed the way it was. I think this is the case for Apple’s view controller containment APIs for example. We’re supposed to set up the child view controller’s view between the calls we’re supposed to make. But still… the API could surely be reworked to make making mistakes harder.

Designing code that helps preventing mistakes

When you’re writing code you should always be on the lookout for anti-patterns like copy-pasting code a lot, having lots of complex state that allows for incorrect states to be represented, or when you’re writing code that has very specific requirements regarding how it’s used.

As time goes on and you gain more and more coding experience, you’ll find that it gets easier and easier to spot potential pitfalls, and you can start getting ahead of them by fixing problems before they exist.

Usually this means that you spent a lot of time thinking about how you want to call certain bits of code.

Whenever I’m working on a new feature, I tend to write my “call site” fist. The call site means the part where I interact with the feature code that I’m about to write.

For example, if I’m building a SwiftUI view that’s supposed to render a list of items that are fetched from various sources I’ll probably write something like:

List(itemSource.allItems) { item in 
  // ...
}

Of course, that code might not work yet but I’ll know what to aim for. No matter how many data sources I end up with, I want my List to be easy to use.

This method of writing code by determining how I want to use it first can be applied to every layer of your codebase. Sometimes it will work really well, other times you’ll find that you need to deviate from your “ideal” call site but it helps focus on what matters; making sure the code is easy to use.

Whenever I’m designing APIs I think about this post from Dave DeLong.

In particular, this quote always stands out to me:

A great API is kind to all developers who work with it.

Every method you write and every class you design has an API. And it’s a good idea to make sure that this API is friendly to use. This includes making sure that it’s hard (or ideally, impossible) to misuse that API as well as having good error messages and failure modes.

Moving on from API design, if you’re modeling state that mostly revolves around one or more booleans, consider enums instead. Even if you’re modeling something like whether or not a view should animate, an enum can help you make your code more readable and maintainable in the long run.

More than anything, if you think that a certain bit of code feels “off”, “too complex” or “not quite right”, there’s a good chance your intuition is correct. Our code should be as straightforward to understand as possible. So whenever we feel like we’re doing the opposite, we should correct that.

That’s not to say that all complex code is bad. Or that all repetition is bad. Or even that every bit of complex state should become an enum. These are all just flags that should stand out to you as something that you should pay attention to. Any time you can change your code a bit in order to make it impossible to represent an impossible state, or if you can make some changes to your code that ensure you can’t pass bad arguments to a method, that’s a win.

In Summary

Writing good code can be really hard. In this post, I outlined a couple of examples of code that allows developers to make mistakes. There are many ways that code can open a developer up to mistakes, and these usually involve code that has evolved over time, which can mean that blind spots have crept into the codebase without the developer noticing.

Through experience, we can learn to identify our blind spots early and we can defensively write code that anticipates change in a way that ensures our code remains safe and easy to use.

Overall, state is the hardest thing to manage in my experience. Modeling state in a way that allows us to represent complex states in a safe manner is extremely useful. Next time you’re considering writing an ‘if’ statement that compares two or more values to determine what should happen, consider writing an enum with a descriptive name and associated values instead.

What are some common coding mistakes that you have learned to identify along the way? I’d love if you told me all about them on X or Threads.

Latest articles

spot_imgspot_img

Related articles

Leave a reply

Please enter your comment!
Please enter your name here

spot_imgspot_img