The Most Common Issues I've Caught Reviewing Swift

The Most Common Issues I've Caught Reviewing Swift


About the author

Chris Griffith has been a game developer for over 19 years and a mobile developer since 2010. He’s produced dozens of apps and games for brands like Match.com, LEGO, Microsoft, Kraft, Anheuser-Busch and PepsiCo.

Chris has been a member of the PullRequest network since April 2018.


There have been five major versions of the Swift programming language since its introduction to the world at WWDC in 2014. It’s one of the fastest evolving languages, largely due to its open nature and the ability for anyone in the community to propose a change. This also means that in a short span of time the syntax looks pretty different than it did five years ago. One effect of this is seen when reviewing Swift code: sometimes developers use conventions that don’t take full advantage of Swift’s capabilities, or they misunderstand how certain language features are meant to be used. In this post, I’ll cover the top three issues I tend to see in code reviews of Swift.

1. Misunderstanding reference counting and ownership

Swift’s memory management is based around a system known as ARC (automatic reference counting). There is no garbage collection mechanism like those found in other languages and platforms. So a misunderstanding or misuse of the ARC system is a common cause of memory leaks and subsequent crashes. Every object in the Swift runtime keeps a count of the number of references to it that exist. Whenever a reference is removed, the count decreases until it hits zero, at which point the object is deallocated from memory (and the deinit method is called). Back in the old days of Objective-C, this process was very cumbersome, involving strict instructions to the compiler about when to retain a reference and when to ignore one. Swift handles this automatically for us, hence the “A” in ARC.

However, as developers we are responsible for leaving hints in our code to indicate when a reference should be created versus not. The keywords we have at our disposal are strong, weak, and unowned. NOTE: You’ll never type the word strong - it is the default reference position. A strong connection to an object means that it increments the reference count by one.

let someObject: ObjectType // A strong reference to this object

Conversely, a weak reference does not affect the reference count. As a result, weak references automatically convert the object into an optional version of itself (more on optionals later). They are optional because at any time the object could have its other strong references disappear, resulting in deallocation. Optional checking provides a safety net so we don’t try to access something that doesn’t exist anymore.

weak var someObject: ObjectType // A weak reference that is (implicitly) optional
weak var anotherObject: ObjectType? // A weak reference that is properly optional

The last keyword tends to be the most confusing for people. The unowned keyword exists for situations where you want the zero-reference counting of a weak connection, but you don’t want to deal with the hassle of optionals.

unowned var someObject: ObjectType // Unowned, non-optional reference

The trick about unowned is that it should only ever be used when the object being referenced will never be nil during the lifecycle of the object referencing it. It can be useful in situations where two objects need to have references to each other, but you don’t want to create a circular reference. A circular reference exists when you have two or more strong references between objects. If all external references to these objects disappear, they will still exist (eating up application memory) because their reference count is not zero.


class TypeA {
    var b: TypeB
}
class TypeB {
    unowned var a: TypeA
}

By setting up the relationship this way (with only one strong reference connecting a to b), when a is deallocated, b will be as well. This forces you to consider which object is the owner of the relationship. As in, which object is responsible for retaining strong references to other objects, and which should have only weak or unowned references. Having a clearly defined ownership structure in your objects will assist Swift’s memory management and avoid memory leaks.

Another area where this comes into play (and is an issue in many PRs I review) is in closures, which are freestanding functions that can be passed around between objects. Consider the following common pattern for using a closure to make a network call for some data.

func fetchData() {
    service.getData { (data) in
        self.displayData(data)
    }
}

func displayData(_ data: Data) {...}

Because the third line is inside a closure, we have to specify self to provide the context for what method to call. However, what many may not know is that in doing this we are actually creating a strong reference to the object represented by self. This is known as capturing a reference. Not only that, but we’re passing this closure to the service object, so we now have an essentially untracked reference to our object that exists in the ether of the network stack. Imagine a scenario where this call happens on a screen in an application. The network stalls out and the user grows impatient and dismisses the screen. The screen has been removed from the view hierarchy of the app, but it has not been properly deallocated because a strong reference to it is still nested inside that closure. When the closure eventually finishes, it will attempt to perform methods on the object outside of its intended context, which in a best case scenario is inefficient and could affect performance of the application and in a worst case scenario will cause a crash.

There’s a fairly simple solution to a problem like this. When a closure is performing an asynchronous operation with an unspecified run time (like a network call or processing a lot of data on a background thread), it’s a best practice to capture self using weak, which will prevent the reference count from being incremented. Consider the modified code from above:

func fetchData() {
    service.getData { [weak self] (data) in
        self?.displayData(data)
    }
}

func displayData(_ data: Data) {...}

Inside the closure, we can still access the original object, but now we have the safety that optionals provide. In the scenario where the user dismisses the screen making this call, the screen will be deallocated and whenever the closure finally runs, the self? will resolve to nil and the code will essentially be skipped.

2. Unwrapping optionals

Another issue I encounter commonly in code reviews is improper handling of optionals. Optionals take a type and wrap it in a special enumeration, which permits it to have a valid value of nil. For instance:

var a: Int // Compiler defaults to 0
var b: Int? // Compiler defaults to nil

The value of a must always be an Integer. Conversely, b may also be nil if no value exists. Optionals have myriad uses throughout Apple’s frameworks and there are many good reasons for them to exist. The process of determining whether an optional is nil or contains a value is called unwrapping. You can imagine it like Schrödinger’s famous cat; until you unwrap an optional, you don’t know what’s inside. There are a few different ways to do this. We’ll start with the two recommended, safe ways to do it and end with two ways that run the risk of a runtime crash.

if/guard Unwrapping

When you’re dealing with optional values, you can use the control flow statements if and guard to safely determine whether a value exists. Consider the following code:


func someMethod(myObject: Object?) {
    if let unwrappedObject = myObject {
        // unwrappedObject is now of type Object and is safe to use
    }
    // OR
    guard let myObject = myObject else { return }
    // For the remainder of this context, myObject is now of type Object
}

In the if statement, we create a new value that safely unwraps the optional. If it is nil, the code in the if block won’t run. An alternate way is to use a guard statement. It essentially does the same thing, but instead of the new non-optional value only being available in the if block, it is available for the remainder of the context (method, closure, etc) that is currently running. I also illustrated an additional concept here called shadowing. In the case of the if statement, I created a new reference called unwrappedObject, and myObject still exists as an optional. In the case of the guard statement, I re-used the same name as the original object, which obscures the optional and redeclares it for the remainder of the context as non-optional.

Optional Chaining

If you don’t need the boilerplate of if/guard statements and just want to call a method or assign a property of an optional object without checking, there is a safe way to do it known as optional chaining. This basically tells the compiler and runtime “This might be nil, and if it is, just ignore everything that comes after it.” This is done by tagging the ? operator at the end of an optional reference.

var myObject: Object?
myObject?.someMethod() // If myObject is nil, this just won't run

It’s called a chain because you can do this for however many levels deep the optionals run.

var myObject: Object?
myObject?.someChildObject?.someMethod()

Implicit Unwrapping

Now we’re entering less safe territory when it comes to unwrapping optionals. Implicit unwrapping is when you unwrap an optional immediately upon declaring it, so you don’t have to unwrap it for the remainder of the context (usually a class).

class MyClass {
    var myObject: Object!
}

This is telling the compiler “I know that nil is a valid value for myObject, but I am 100% sure this will never be nil. Or if it is, I’m comfortable with this crashing.” I recommend this technique ONLY be used with IBOutlets in apps. IBOutlets expose properties to the Interface Builder tools inside Xcode. They are almost always references to view objects which will exist alongside a view controller or similar structure. Because of how IB files are inflated and wired to their controller counterparts, it’s not possible to make these outlets non-optional. As such, a common pattern is the following:

class ViewController: UIViewController {
    IBOutlet var label: UILabel!
}

For the context of this controller’s lifecycle, barring unforeseen circumstances, the label should never be nil and can therefore benefit from the cleaner syntax of non-optionals. However, due to a misguided decision on Apple’s part, when you wire up IBOutlets using Xcode’s drag-and-drop tools, what it creates by default is

IBOutlet weak var label: UILabel!

Note that this is no longer a strong reference, meaning it could be deallocated without warning. The thinking is ostensibly that because the label’s view parent also holds a reference to the label, this shouldn’t be an issue. However, what if there is logic that removes the label from the view hierarchy? Or what if Apple changes view lifecycle execution order in a future version of iOS? Suddenly, it is deallocated and code that relies on the implicit unwrapping above will fail and trigger a crash. Because this is inherently unsafe, I always recommend the following:

IBOutlet var label: UILabel! // Implicitly unwrapped, strongly referenced
// or
IBOutlet weak var label: UILabel? // Properly optional, weakly referenced

Both of these are valid and should be sufficiently runtime safe to avoid crashes. Technically, the latter is the safest option, since it enforces unwrapping every time the label is referenced, but the lack of overhead of the first one is satisfactory for me and never caused a problem.

Force Unwrapping

The last way to unwrap an optional is to use The Force™. Not really, though that would be awesome. The syntax is similar to implicit unwrapping, but instead of using it at the declaration site, you use it whenever you reference an optional. It tells Swift “I live on the edge and do things like drive without a seat belt or swim with hungry sharks. As such, I don’t care if this value could be nil. Unwrap this thing fast so I can get back to playing darts with my eyes closed.” If the value of a force unwrapped optional is nil, the app will crash. Do not pass Go, do not collect $200.

var myObject: Object?
// Some time later
let thisCrash = myObject!.someValue

While it’s important for this operator to exist, it is extremely unsafe, and I will always call it out in a code review. With the other options available for preventing crashes, force unwrapping feels like laziness and/or arrogance. Just don’t do it in production code.

3. The Power of Codable

In Swift 4, Apple finally answered the many voices crying out for a replacement for the aging and kludgy NSCoding system from Objective-C. The Codable protocol (which is actually two protocols, Encodable and Decodable) allows you to provide a blueprint for your custom types that can be used for serialization to any number of formats (XML, JSON, etc). Probably the most common use of Codable is for data objects that are parsed from network-provided JSON. However, something I see all too regularly is when developers provide a ton of custom implementation they don’t need. Codable is intended to be powerful and simple. Consider the following code:

struct User: Codable {
    let name: String
    let id: String
    let avatarUrl: String?
}

This basic user object can be provided to a JSONDecoder when there is a JSON blob that matches its schema. In fact, assuming these names match what’s in the JSON, you can stop here. Seriously. It’s that awesome.

However, let’s say that like many back-end services, the JSON doesn’t have camelCase names. It has snake_case instead. We don’t want to have to put avatar_url in our Swift code - that’s just gross and goes against convention. We have two options in front of us.

  1. We can provide a CodingKey enum as a child of our User type. This provides Swift with a mapping between server names and client names. This isn’t much extra work and provides options for when the JSON names could be improved by changing more than just their case.

struct User: Codable {
    let name: String
    let id: String
    let avatarUrl: String?

    enum CodingKeys: String, CodingKey {
        case name, id // You only need list the case when the name is the same
        case avatarUrl = "avatar_url"
    }
}
  1. An option not many people seem to know about involves configuring the JSONDecoder object. If you can rely on all of your JSON names being in snake_case and all your Swift names being camelCase, you can add a single line to your decoding to make everything happen automagically:
let decoder = JSONDecoder()
decoder.keyDecodingStrategy = .convertFromSnakeCase

In the event that your JSON data is less predictable or that you want to have data types in your User object that aren’t natively supported by Codable, like URL. In cases like this, you can implement the init(from: Decoder) method.


struct User: Codable {
    let name: String
    let id: String
    let avatarUrl: URL?

    enum CodingKeys: String, CodingKey {
        case name, id // You only need list the case when the name is the same
        case avatarUrl = "avatar_url"
    }

    init(from decoder: Decoder) throws {
        let container = try decoder.container(keyedBy: CodingKeys.self)
        name = try container.decode(String.self, forKey: .name)
        id = try container.decode(String.self, forKey: .id)
        if let url = try? container.decodeIfPresent(String.self, forKey: .avatarUrl) {
            avatarUrl = URL(string: url)
        }
    }
}

Note that this can be cumbersome for large data models and I definitely don’t recommend implementing this unless you absolutely have to. Because of all the messy try syntax, it’s easy to introduce an error that will be painful to debug. The rule to follow with Codable should be to setup only as simple as possible; let the Swift compiler do the heavy lifting.

Bonus: Enums in Codable

We’ve covered a lot in this post, but I couldn’t resist mentioning one more thing about Codable and one of my favorite elements of Swift programming: Enumerations.

Enums are incredibly powerful tools for organizing data and simplifying logic. I could write a whole post on all the things you can do with them in Swift, but what I want to mention is something I think many people aren’t aware of. You can use enums with Codable!

Returning to the User object example from above, let’s consider how we might add a user state field to this object. The typical way would be something like this:

struct User: Codable {
    let name: String
    let id: String
    let state: String
}

The biggest problem with this approach is how to store the various values for state, since we will almost certainly need to implement control flow based on it. If we know the possible values for it, we can build a string-based enumeration to match.

enum UserState: String {
    case unverified
    case verified
    case inactive
}

I’ve seen a number of people get this far in their code and then they stop. They create some sort of mapping from the string value to the enum. But this is also prone to mistakes and isn’t taking full advantage of Swift. If we simply make UserState conform to Codable, we can use it in our User object.

struct User: Codable {
    let name: String
    let id: String
    let state: UserState
}

enum UserState: String, Codable {
    case unverified
    case verified
    case inactive
}

Voila! We’ve now defined two very simple types which are easy to reason about and Codable will take care of all the heavy lifting for us. Note that if UserState might have future possibilities added to it, this would be a good case to add an unknown case to the enum and implement a custom init, which defaults to that case if it’s unable to parse the value.

Wrap up

Swift is going to continue to evolve and get new features. It’s important to leverage its toolkit to the fullest extent possible. It will not only reduce the amount of code you have to type, but make that code much safer to run.


About PullRequest

HackerOne PullRequest is a platform for code review, built for teams of all sizes. We have a network of expert engineers enhanced by AI, to help you ship secure code, faster.

Learn more about PullRequest

Chris Griffith headshot
by Chris Griffith

February 24, 2020