The Danger of Playing it Safe

It’s an old chestnut that Swift developers love to sink their teeth into: Should you force unwrap optionals? Everyone has an opinion about it, and I’m going to state mine as clearly as I can — forcefully unwrap early and often.

Of course, this is just one opinion. We even disagree about it within our two man Agenda team, so there is probably not much hope for the world. But I want to explain when and why I think it is good to force unwrap, and give a recent real world example that demonstrates the cost of not doing it.

First, my reasoning. I think you should force unwrap in any situation where logic tells you that an optional simply cannot be nil. A standard example might look like this.

var count: Int?
count = 0
print("\(count!)")

Could count end up being nil in this case? Perhaps, through some strange memory corruption or solar flare. Do I want my app to continue to run if count is nil? No. To put it bluntly, that is cancer. The app would be in an undefined state, which may well be dangerous. I don’t want it deleting user data or causing any other havoc — it should just crash. I will be informed of the problem, and it can’t do any more damage.

But what is wrong with the softly, softly approach? Shouldn’t we be defensive here? Here is how you could do it.

var count: Int?
count = 0
if let count = count {
    print("\(count)")
} else {
    print("error!")
}

There are several problems with this. Firstly, it is more verbose, making it more difficult to read. Secondly, and even more importantly, it disguises developer intentions. It says to the next developer “count could conceivably be nil, so we will check for it”, even though it was clear to the original developer that it should never be nil in any well-defined circumstance. Lastly, where is the app at after this code runs? Who knows? The state is completely unclear.

So be assertive with forced unwrapping. If there is no case where the optional should ever be nil, force unwrap. But don’t force unwrap in cases where there is a valid situation in which nil could arise. If a foreseen error could lead to nil, that requires explicit handling.

I mentioned that I wanted to finish off with a recent real-world example. When Apple announced iOS 14 would ship, they only gave about a day’s notice, leaving most of us scrambling to get ready. For Agenda, we had done a lot of work on widgets, but we had never had them in a beta, or extensively tested on the device. That was planned for the week or so that we usually have following release of the Xcode GM, but before the new iOS is available. Alas, in 2020, that was a luxury we didn’t have.

So I merged our branches in Git, and uploaded the binary. Downloaded from TestFlight, and — you guessed it — the widgets were broken. It was just showing a placeholder, which I later learned is what you get when a widget crashes.

I dug into the code, and I found that the problem was an incorrect file path. I had been testing with a special development target, and hadn’t properly tested the production target. The faulty code was as follows:

let identifierSuffix = (Bundle.main.object(forInfoDictionaryKey: "AgendaIdentifierSuffix") as? String) ?? ".development"

It is attempting to get a string from the Info.plist, which it will then use in the aforementioned path, but I had forgotten to put the string in the Info.plist. That wasn’t the real crime though: As you can see, in the case of nil being returned, it tries to play it safe by falling back on the development target’s string.

This was bad. It meant the production app had the wrong path, so it crashed, but I didn’t see the crash until very late in the development cycle, even though it was there the whole time. And the cost? I probably wasted an hour or two uploading the binary, then sluthing the problem. “What’s an hour or two?” I hear you ask. Well, Apple were pushing us to submit our new widget feature so that Agenda could be considered for featuring. I don’t think two hours was enough to jeopardize that, but you never know. I’m sure the App Store is a madhouse on release day.

Here is how I think the code should have been written.

let identifierSuffix = (Bundle.main.object(forInfoDictionaryKey: "AgendaIdentifierSuffix") as? String)!

It is simpler, clearly stating that I am assuming the entry is in the Info.plist, and the widget only works if that assumption is fulfilled. If there is no entry in there, the widget is entirely useless, and should crash before causing more damage. And, with this code, it will crash early in development. I would have discovered the issue as soon as I tried to run a debug version, and fixed it long before it hit the App Store.

(Update: It has been pointed out that the last line of code is a bit convoluted. It would read better as

let identifierSuffix = Bundle.main.object(forInfoDictionaryKey: "AgendaIdentifierSuffix") as! String

That’s very true. I wanted to change the original code as little as possible, to stay on message. The point is the same: defensive coding did more damage than good, and an assertive force-unwrap would have been better.)

8 thoughts on “The Danger of Playing it Safe

    1. Thanks for the comment, Adam.

      > you can succinctly document cases where nil is not expected behavior

      Isn’t that what force unwrap does? When you use it, you are saying “I never expect this to be nil”. If I did expect cases where it was, I would never use it. That would be suicide.

      The other question is, what does the extension actually buy you? You still crash, and get a crash log presumably. I assume the code in question is very similar to whatever the compiler itself uses for force unwrap, and the message you pass in will state the obvious (Eg. “Variable x was unexpected nil”. Dah.)

      1. The difference is in DEBUG mode, it stops in the debugger, where you could presumably fix the code, change the variable a replacement value temporarily, and continue (fix and continue).

  1. I’m still not a fan of forcing it and getting silent crashes. I usually go with a 3rd option of:

    var count: Int?
    count = 0
    if let count = count {
    print(“\(count)”)
    } else {
    fatalError(“FatalError: Count was nil”)
    }

    usually in a guard though. I’ve found that having the extra log to be helpful in addition to the crash log when either myself or my QA guys are testing.

    I also think it’s more explicit in my intention when looked at for future development. I’m specifically stating that it should never be nil and that it’s really bad if it is. If I inherited code with a force unwrap I don’t know for sure if the previous dev’s intention was to crash due to it being fatal or if it was a dev with bad practices that didn’t account for nil.

  2. On large teams force unwrapping takes away the guard rails and allows novice developers to crash the app too easily. In code review it’s also hard to spot the force unwraps. The correct 🙂 approach is the one Brad showed using fatalError so you can disable allowing force unwrapping via swiftlint and still make mandatory data required. We’ve tried many different approaches on various projects and the fatalError approach yields the fewest unintentional bugs while still giving the benefits of force unwrapping.

    1. This only makes sense in cases where the variable should have been optional to begin with. In any case where forced unwrapping is appropriate, your app now has a nil value where a nil value was not expected. Depending on the situation, anything could happen. The novice developer wrote the algorithm without accounting for this scenario.

      1. Using the fatalError approach to unwrap in a guard or if statement is exactly the same as force unwrapping (it always crashes) except that it makes the intent clear in code review (which force unwrap does not because it’s hard to spot when scanning code).

Leave a Reply to Drew McCormack Cancel 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