What is the proper use of guard when testing for the existence of a property

Question

In my code I am being passed a dictionary. That dictionary can contain an error string. Like this

func handleSomething(_ json: [String: Any]) -> (Bool, String?) {
    if let error = json["error"] as? String {
        print("I have an error: \(error)")
        return (false, error)
    }
    ...
    return (true, nil)
}

I would think this would be a good place to use guard, however the code is not as concise

func handleSomething(_ json: [String: Any]) -> Bool {
    let error = json["error"] as? String
    guard error == nil else {
        print("I have an error: \(error!)")
        return (false, error)
    }
    ...
    return (true, nil)
}

Is there a better way to do this so the assignment and check can be done in one line? If not is this the correct use of guard?

Update: Added return values to better illustrate the real intention of the question.

just keep the first variant and don't think too much about it. — Sulthan, Sep 26 '17 at 12:28
No need to use guard and/or if conditional. Just return `json["error"] == nil` — Leo Dabus, Sep 26 '17 at 12:35
Not all early exits need to be done with `guard`; using `if let` here is also the way I'd do it. — Hamish, Sep 26 '17 at 12:39

score 2 · Accepted Answer · answered Sep 26 '17 at 12:42

Generally you should strongly avoid passing around [String: Any]. You should parse this down to a stronger data type earlier, and you should separate out error cases there, often with an enum:

struct ServerData {
    // good data; no errors
}

enum ServerResponse {
    case success(ServerData)
    case error(String) // Return this if error is set
}

With this, our functions would just take ServerData mostly, so errors have already been dealt with. Whenever possible, rather than coming up with clever workarounds to problems, make the whole problem go away.

That said, there's still the parser itself to deal with and this situation can come up both there and even when you have stronger types. In many cases you wind up with this guard/log pattern very often, and it's definitely worth avoiding that. So pull out a function:

func noErrorOrLog(_ json: [String: Any]) -> Bool {
    if let error = json["error"] as? String {
        print("I have an error: \(error)")
        return false
    }

    return true
}

Now actual handling functions can use guard and avoid duplicating the logging:

func handleSomething(_ json: [String: Any]) -> Bool {
    guard noErrorOrLog(json) else {
        return false
    }
    // ...
}

In some cases this doesn't make sense (if you're doing it exactly once perhaps), in which case if-let is the right tool. Creating confusing code to create a guard is counterproductive. But in many cases you can avoid this problem entirely (by filtering errors earlier with strong types), or avoid duplication by moving to a function.

I would agree this is better, however what if (and in my real code) I want to pass back the error code (using a return statement or callback). In my production code this is the earliest point the error can be handled (response from API) — datinc, Sep 26 '17 at 13:24
Personally I'd probably use `throws` there, which makes a `try noErrorOrThrow(payload)` function very easy. No need for a guard. — Rob Napier, Sep 26 '17 at 15:53

What is the proper use of guard when testing for the existence of a property

1 Answers1