You're probably familiar with Swift's nil-coalescing operator. The nil-coalescing operator returns a fallback value if the statement on its left doesn't produce a value. I use it quite often and there is nothing inherently wrong with it. The code smell we discuss in this episode of Code Smells in Swift is the value the statement on the right of the nil-coalescing operator produces, the fallback value.
Meaningless Fallback Values
Fire up Xcode and create a playground using the Blank template from the iOS section.
Clear the contents of the playground and add an import statement for the Foundation framework.
import Foundation
We define a struct with name Book that conforms to the Decodable protocol. It declares three properties, id of type Int, title of type String, and author of type String. Note that the properties of the Book struct have a non-optional type, which means a Book object is required to have an id, a title, and an author.
import Foundation
struct Book: Decodable {
// MARK: - Properties
let id: Int
let title: String
let author: String
}
Even though conformance to the Decodable protocol is generated for us, we implement the required initializer of the Decodable protocol. We use the initializer of the Decodable protocol to illustrate the code smell. We create an extension for Book and declare a private enum with name CodingKeys that conforms to the CodingKey protocol. It defines the coding keys for the Book struct. The raw values of CodingKeys are of type String. We define a case for each property.
extension Book {
// MARK: - Types
private enum CodingKeys: String, CodingKey {
case id, title, author
}
}
Implementing the initializer is straightforward. We ask the decoder for a keyed container and use the container to decode the values for the id, title, and author properties.
extension Book {
// MARK: - Types
private enum CodingKeys: String, CodingKey {
case id, title, author
}
// MARK: - Initialization
init(from decoder: Decoder) throws {
let container = try decoder.container(keyedBy: CodingKeys.self)
id = try container.decode(Int.self, forKey: .id)
title = try container.decode(String.self, forKey: .title)
author = try container.decode(String.self, forKey: .author)
}
}
Let's assume the books are fetched from a remote API. We define a string literal and convert it to a Data object to mock the response of the remote API.
let data =
"""
{
"id": 1,
"title": "The Missing Manual for Swift Development",
"author": "Bart Jacobs"
}
""".data(using: .utf8)!
To convert the Data object to a Book object, we create a JSONDecoder instance and pass the Data object to the decode(_:from:) method. We instruct the JSON decoder to decode a Book object from the Data object.
let data =
"""
{
"id": 1,
"title": "The Missing Manual for Swift Development",
"author": "Bart Jacobs"
}
""".data(using: .utf8)!
let book = try JSONDecoder().decode(Book.self, from: data)
print(book)
The decoding operation succeeds without issues and the Book object is printed to the console.
Book(id: 1, title: "The Missing Manual for Swift Development", author: "Bart Jacobs")
Let's modify the response of the remote API by omitting the id field.
let data =
"""
{
"title": "The Missing Manual for Swift Development",
"author": "Bart Jacobs"
}
""".data(using: .utf8)!
let book = try JSONDecoder().decode(Book.self, from: data)
print(book)
The result of this change isn't surprising. The decoding operation fails since the id property is required to create a Book object.
Playground execution terminated: An error was thrown and was not caught:
▿ DecodingError
▿ keyNotFound : 2 elements
- .0 : CodingKeys(stringValue: "id", intValue: nil)
▿ .1 : Context
- codingPath : 0 elements
- debugDescription : "No value associated with key CodingKeys(stringValue: \"id\", intValue: nil) (\"id\")."
- underlyingError : nil
If this happens in production, your application may display an error to the user or, even worse, nothing. This is something we need to avoid so we need to prepare for this scenario. The problem is that the precautions developers take tend to be inappropriate. Take a look at the following solution.
extension Book {
// MARK: - Types
private enum CodingKeys: String, CodingKey {
case id, title, author
}
// MARK: - Initialization
init(from decoder: Decoder) throws {
let container = try decoder.container(keyedBy: CodingKeys.self)
id = (try? container.decode(Int.self, forKey: .id)) ?? 0
title = (try? container.decode(String.self, forKey: .title)) ?? ""
author = try container.decodeIfPresent(String.self, forKey: .author) ?? ""
}
}
The updated initializer contains two problems. First, the initializer assumes the id, title, and author fields may be null or missing. For that reason, the initializer uses the try keyword with a question mark and, for the author property, the decodeIfPresent(_:forKey:) method. This is a code smell because the declaration of Book defines the three properties as non-optional, that is, required. Second, because the initializer uses the try keyword with a question mark and the decodeIfPresent(_:forKey:) method, it uses the nil-coalescing operator with a fallback value. The fallback values are the code smell I would like to discuss in more detail.
What does it mean for a book to have 0 for an id or an empty string for title or author? The fallback values that are used in the initializer have no meaning. They are necessary because the initializer uses the try keyword with a question mark and the decodeIfPresent(_:forKey:) method.
Falling back to values that have no meaning is a code smell and should be avoided if possible. Because the application doesn't crash and no error is thrown, issues caused by meaningless fallback values can go unnoticed for quite some time. They are difficult to debug and usually result in a poor user experience.
Fixing the Code Smell
The solution is plain and simple. We revert the initializer to its original implementation. If the Book struct reflects the API contract, that is, the id, title, and author fields are required for a book, then the decoding operation should fail if one or more required fields are missing from the response returned by the remote API.
extension Book {
// MARK: - Types
private enum CodingKeys: String, CodingKey {
case id, title, author
}
// MARK: - Initialization
init(from decoder: Decoder) throws {
let container = try decoder.container(keyedBy: CodingKeys.self)
id = try container.decode(Int.self, forKey: .id)
title = try container.decode(String.self, forKey: .title)
author = try container.decode(String.self, forKey: .author)
}
}
Decoding a response that doesn't include the id field results in an error and that is an appropriate response from the client. The remote API violates the contract it has with the client and that needs to be addressed on the backend. It is important to log errors like this to a remote server to make sure issues like this can be detected and addressed as soon as possible.
Defensive Coding
It is true that the code you write needs to be flexible and forgiving, but there are limitations. We could change the properties of the Book struct to be of an optional type. That would also address the code smell. That solution isn't ideal, though. A responsibility of the backend, response validation, is pushed to the client. Every client would need to take on that responsibility and the implementations would need to be aligned. That is something you need to avoid.
A proper API contract is indispensable. The API contract defines the type and nullability for every field of the response. Both parties need to enforce the API contract and that means the backend needs to validate the response it returns to the client.
What's Next?
The nil-coalescing operator is convenient, but it is important that you provide a fallback value that makes sense. An empty string is very often a code smell. It needs to be replaced with nil or a value that makes sense. If you don't have access to a value that makes sense, then throw an error.
