3

Often I need to transform a type to another, such as a networking model to a data model, or a data model to a binary representation. Should these transformation functions take an Optional/nullable value and immediately return nil if it's nil, or should the function only accept non-nil values and always return a non-nil transformation? Assume my code properly handles validation when receiving data, so no further validation is needed, just transformation.

Example 1: Have the caller handle nil objects

func makeUser(from apiUser: APIUser) -> User {
    return User(id: apiUser.id)
}

var user: User? = nil
func downloadUser() {
    service.getUser { (apiUser: APIUser?) in
        if let apiUser = apiUser {
            self.user = makeUser(from: apiUser)
        } else {
            self.user = nil
        }
    }
}

Example 2: Have the transform handle nil objects

func makeUser(from apiUser: APIUser?) -> User? {
    if apiUser == nil {
        return nil
    }
    return User(id: apiUser.id)
}

var user: User? = nil
func downloadUser() {
    service.getUser { (apiUser: APIUser?) in
        self.user = makeUser(from: apiUser)
    }
}

Example 1 explicitly states an APIUser will always transform to a User object. While Example 2 implicitly implies that a valid APIUser may not map to a valid User object, even though that is guaranteed.

While Example 2 does demonstrate that the common logic is handled in the transform instead of copied each time it's needed, I personally don't like the fact that the transform can return an optional/nullable value even though my function contract states validation has already been performed and a valid value will never fail.

1
+50

If the only purpose of an optional parameter is to return nil if the parameter is nil then I wouldn’t do it. Get some additional type safety by making the parameter non-optional. Then anyone having a possibly nil value can check for that. Quite often it turns out the value is actually never nil and there is no checking code in the end.

0

The makeUser function takes a value and creates a user out of it. If it cannot create a user because the input is invalid, it should throw an exception. Null is definitely an invalid input as you can't parse "nothing" and get a User.

This is similar to parsing an int. If the input is invalid, throw an exception, don't pretend it went well and then return the default value.

If you do this, then whenever you call makeUser you know that you definitely have an actual User object, not null.

downloadUser should then validate that the user it downloaded is valid (in your case that seems to just be checking that it isn't null).

While it doesn't particularly matter for this one use case, the benefit is that other calls to makeUser either work (in which case you have a user object) or have to explicitly be handled by the calling code. This helps reduce the chances of null reference exceptions throughout the app.

EDIT: For summary, Example 1 is better to me.

  • In Swift you don’t throw exceptions all that often. Returning nil is quite appropriate. Whether the argument should be optional at all, that is the question. – gnasher729 Oct 22 '19 at 11:29
  • In swift parsing an String to Int will not throw, it will return nil. developer.apple.com/documentation/swift/int/2927504-init. I prefer to only use throw for exception handling and not for control flow. I do agree that I rather my function have non-optional, just I rather not throw. – Michael Ozeryansky Oct 22 '19 at 15:43
  • You wouldn't be throwing to control flow. You would be throwing to tell the calling code "You need to pass valid input, or you need to not call me". That forces the calling code to handle this using an if statement (like you had in your example) rather than leaving it to chance. When the app is running, no exception will, as the calling code will check input first (like in your example). This is a defensive exception for future calling code to let the developer know what they should and should not pass to the method. Regardless, I don't know enough about swift, so sorry for that! – Jack Oct 22 '19 at 16:42
  • @Jack From the example, it's possible and legal for the value from the network layer to be nil. I believe, if the caller changes it's control flow based on the catch then it's control flow and not exception handling. Here, I'm interested in where to handle the nil check. In the function or outside of the function. – Michael Ozeryansky Oct 22 '19 at 18:28
  • I agree that it is legal for the network layer to be nil. I was suggesting that this is not a legal value to be nil in makeUser. I didn't suggest a catch statement, so i'm a little unsure where that assumption has come from. makeUser can throw an unhandled exception to force the developer to put the if statement in. As I said, i'm unsure on best practices in swift, I'm basing this anwser (and comments) entirely on trying to avoid the use of null's as much as possible, and in my eyes a function called makeUser should never return null (as if it returned null, it didnt make a user.) – Jack Oct 23 '19 at 10:34
0

The APIUser, being a DTO, exists at the boundary of your application. It should not be leaking out beyond the object that interacts with the remote service. Neither example is the right solution. The real problem lies with your service not returning a User object and performing the mapping from APIUser to User internally.

As for nullability? It's a remote system. You do not control it. It can be null.

Your service should be refactored to return a User?. If the API returns a null value, the service should return a nil. You need to give your application a chance to handle situations where the user does not exist in the remote service. This might be an innocent mistake where someone tried to log in, but they don't have a user profile yet. No need to crash the application when a polite "You do not have permission to access this application" message would suffice.

So basically the answer is Example 3 (below)

// 'service' calls API and maps to User domain object
var user: User? = service.getUser()

if user == nil {
    // user not found in remote service, handle error
}

This is honestly no different than querying a database for your application. The user might not exist. Return a nil instead of crashing the application with an exception you probably need the user to handle anyhow.

  • I don't think you understood the example. In both cases I show that the service does in fact return an optional User object. The question is how to handle the optional object after the service returned it. Either way, you didn't address the question. Which is whether the transform function should handle nil or the optional should be handled by the caller. – Michael Ozeryansky Oct 22 '19 at 15:35
  • @MichaelOzeryansky: There shouldn't be a transform function outside of the service. That was the main point of my answer. The service should return a User? and the mapping of APIUser to User should be a concern for the service, not the code using the service. – Greg Burghardt Oct 22 '19 at 15:38
  • APIUser is a network object, and the User is a data model. These exist within separate layers of the framework. Do you recommend having both the APIUser and the User object defined in the network layer? If the server returned malformed data, I would normally handle that with a Result type returning .failure, not a nil User. – Michael Ozeryansky Oct 22 '19 at 15:39
  • @MichaelOzeryansky: Can you include more context about where this "makeUser" function is defined and where it is being called? I feel like there is an architectural issue to resolve here. – Greg Burghardt Oct 22 '19 at 15:50
  • Regardless of where it is, I asked this question to address whether transformer functions should take a nil value. The network example is just an example. There are many other places where I need to transform a types, such as going from Model to ViewModel. – Michael Ozeryansky Oct 22 '19 at 18:25

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.