Chill Out with the Defaults

I predominantly work in Swift and Kotlin, both of which support default arguments. As with any feature it’s worth being careful as overuse can lead to unexpected design trade-offs.

A common pattern I keep seeing in various codebases I work on is that data transfer objects are being defined using default arguments in their constructors. I think this leads to a few issues that I’ll explore in this post.

A Simple Example

Here’s a typical example of a class with a default argument on tags.

struct BlogPost {
    let title: String
    let tags: [String]

    init(title: String, tags: [String] = []) {
        self.title = title
        self.tags = tags
    }
}

data class BlogPost(
    val title: String,
    val tags: List<String> = emptyList()
)

It’s not always clear why this is done. I suspect it’s often out of habit or convenience for testing. My suspicion that this is related to testing comes from seeing this pattern repeatedly even though the production code is explicitly providing every argument and therefore never makes use of the defaults but the tests do. It gets my spidey senses tingling when it feels like we are weakening our production code in the service of adding tests.

The unintended consequence of these defaults is that the compiler can no longer be as helpful.

Exhaustivity Tangent

Just to make sure we are on the same page let’s talk about exhaustivity checking with enums as hopefully people will have experience with this. If we declare an enum and later switch over its cases the compiler can check to make sure we cover every case (assuming we don’t add a default case.)

For example let’s start with a PostStatus with two cases

enum PostStatus {
    case draft
    case published
}

func outputFolder(status: PostStatus) -> String {
    switch status {
        case .draft: "/dev/null"
        case .published: "blog"
    }
}

enum class PostStatus {
    Draft,
    Published
}

fun outputFolder(status: PostStatus): String {
    return when (status) {
        PostStatus.Draft -> "/dev/null"
        PostStatus.Published -> "blog"
    }
}

If we add a third case of archived then the compiler will force us to revisit our outputFolder function as it’s no longer exhaustive:

enum PostStatus {
    case archived
    case draft
    case published
}

func outputFolder(status: PostStatus) -> String {
    switch status { // Error -> Switch must be exhaustive
        case .draft: "/dev/null"
        case .published: "blog"
    }
}

enum class PostStatus {
    Archived,
    Draft,
    Published
}

fun outputFolder(status: PostStatus): String {
    return when (status) { // Error -> 'when' expression must be exhaustive. Add the 'Archived' branch or an 'else' branch.
        PostStatus.Draft -> "/dev/null"
        PostStatus.Published -> "blog"
    }
}

This is great because it means the compiler will guide us step by step through every callsite so we can decide what the appropriate action to take is.

Missing Exhaustivity 😢

If we agree that exhaustivity checking is a good thing then we can extend the same logic to the first example. Let’s say we create instances of our BlogPost type in a few places in our codebase and we want to add a new property of isBehindPaywall. If we add a default value then the compiler doesn’t help us by highlighting all the callsites that we should reconsider. If we are lucky then we make the change and all is fine, if we aren’t so lucky then we could accidentally have blog posts being hidden/shown when they are not supposed to be. In this case I’d much rather the compiler makes me check every callsite so I can make the correct decision.

In practice all this means is that I have to explicitly specify the isBehindPaywall argument and accept that there might be some duplication:

// Explicit
BlogPost(title: "Some blog post", isBehindPaywall: false)

// Implicit
BlogPost(title: "Some blog post")

// Explicit
BlogPost(title = "Some blog post", isBehindPaywall = false)

// Implicit
BlogPost(title = "Some blog post")

Local Reasoning

The explicit version above has another strength to it that is due to the improved local reasoning. If I want to know how the isBehindPaywall state was decided I can simply look at the callsite that instantiated the instance. In the defaults case this isn’t as simple - first I need to look at the callsite then if no value was provided I need to look up the declaration of BlogPost. With IDEs that click through this might not seem like a hardship but it also means we are vulnerable to changes being made at a distance e.g. someone could change the default value and it could have wide ranging side effects to any callsite that didn’t explicitly add a value.

Discoverability

You might think it’s fine when I add the property I’ll go and check every callsite myself manually. This is all well and good if you are the sole owner of the codebase and if you aren’t publishing your code as a library. But bear in mind it’s not a permanent fix as anyone can come along and create an instance of BlogPost and they may or may not see the isBehindPaywall option, which means they could get it wrong.

The issue is when people come to create instances of our BlogPost type they can get away without providing a value for isBehindPaywall and be blissfully unaware it even exists or its impact e.g.

BlogPost(title: "My Blog post")

BlogPost(title = "My Blog post")

Respecting Boundaries

Another subtle issue is whether something like a data transfer object should even have this knowledge bestowed upon it or if some code that has the business rules should be in charge.

Consider this scenario I had recently:

I have a backend service that supplies data for Android and iOS clients. The backend uses kotlinx.serialization, iOS uses some legacy JSONSerialization code and Android is using Gson.

       +---------+
       | Backend |
       | kotlinx |
       +---------+

+---------+  +-------------------+
| Android |  |        iOS        |
|   Gson  |  | JSONSerialization |
+---------+  +-------------------+

With this setup we have 3 different code bases that are bound by an informal contract of what the JSON should look like. Each platform is using different libraries to encode/decode and could have subtle differences in how this is done.

We also have a Kotlin Multiplatform library so it makes sense to refactor like this:

• Extract the code from the backend service to the Kotlin Multiplatform module
• Utilise this kotlinx.serialization based code from all 3 places

       +-----------------------+
       | kotlinx.serialization |
       +-----------------------+
            ^      ^      ^
           /       |       \
          /   +---------+   \
         |    | Backend |    |
         |    +---------+    |
         |                   |
       +---------+    +---------+
       | Android |    |   iOS   |
       +---------+    +---------+

We still have 3 code bases that have to agree on how the JSON is structured but now that is handled in a more concrete way by providing the type and the encoding/decoding logic in a shared library.

With this refactor originally the type that lived in the backend service had default values encoded into it but with this new split it doesn’t really make sense. The Android/iOS clients are supposed to be totally dumb and just trust the data handed to them but the original type knows too much with defaults being baked in. It makes much more sense to strip the defaults and keep the business rules on the server populate these values, which means that the type is a simple as possible.

Conclusion

It may seem like I’m beating on default arguments but in reality I use them all the time. My main point is before adding defaults, ask what you might lose. Sometimes, explicit arguments add a little duplication but make your code safer, more discoverable and easier to reason about.