I have this code to access travel time from the Google directions api JSON result:

let jsonResult = try JSONSerialization.jsonObject(with: data, options:JSONSerialization.ReadingOptions.mutableContainers) as! NSDictionary

                DispatchQueue.main.sync
                    {
                        if (jsonResult.value(forKey: "status") as! String == "OK")
                        {

                            if let routes = jsonResult.value(forKey: "routes") as? NSArray
                            {

                                if let legs = routes.value(forKey: "legs") as? NSArray
                                {

                                    if let duration = legs.value(forKey: "duration_in_traffic") as? NSArray
                                    {

                                        for i in 0..<duration.count
                                        {
                                            let timeDuration = duration[i] as! NSArray

                                            if let time = timeDuration.value(forKey: "value") as? NSArray
                                            {

                                                print("Current Time: \(Int(time[0] as! NSNumber)/60) Min")

                                            }
                                        }

                                    }

                                }

                            }

                        }
                }

It works perfectly fine but it looks like really bad code. But every example i found solves it similar. Has anyone a cleaner solution? Thank you in advance!

up vote 4 down vote accepted

Since Swift 1.2, multiple optional bindings and boolean conditions can be combined to a single if-statement. This helps to avoid the "Swift if-let pyramid of doom". In your case that would be

if jsonResult.value(forKey: "status") as! String == "OK",
    let routes = jsonResult.value(forKey: "routes") as? NSArray,
    let legs = routes.value(forKey: "legs") as? NSArray,
    let duration = legs.value(forKey: "duration_in_traffic") as? NSArray {

    for i in 0..<duration.count {
        let timeDuration = duration[i] as! NSArray
        if let time = timeDuration.value(forKey: "value") as? NSArray {
            print("Current Time: \(Int(time[0] as! NSNumber)/60) Min")

        }
    }
}

and reduces the maximal nesting level by three.

There are two "forced unwraps" in your code:

let jsonResult = try JSONSerialization.jsonObject(with: data, options:JSONSerialization.ReadingOptions.mutableContainers) as! NSDictionary

and

if jsonResult.value(forKey: "status") as! String == "OK",

which can cause a program termination at runtime. Better use optional casts. In the second case that would simply be

if jsonResult.value(forKey: "status") as? String == "OK",

because optionals and non-optionals can be compared.

The JSONSerialization.ReadingOptions.mutableContainers option is not needed because you are not modifying the returned dictionaries or arrays.

You are using value(forKey:) a lot. This is a method from the Key-Value Coding protocol, and is often not what you need. In

let routes = jsonResult.value(forKey: "routes")

you are simply retrieving a dictionary value, that is:

let routes = jsonResult.object(forKey: "routes")

In

let legs = routes.value(forKey: "legs")

you are mapping the routes array to a new array legs. This is Key-Value coding, however it can be confusing, in particular in this case where legs would be an array of arrays. This makes duration and timeDuration nested arrays as well, and is the reason why you have to subscript time[0] finally. If you are interested in the first route only, then it is easier to extract the first element of routes and continue to work with that.

Generally it is preferred to use native Swift type instead of Foundation types, e.g. [Any] (or a more specific type) for an array, or [String, Any] for a dictionary. Compare Working with JSON in Swift from the Swift blog for more information.

Instead of

for i in 0..<duration.count {
    let timeDuration = duration[i]
    // ...
}

you can simpler write

for timeDuration in duration {
    // ...
}

Putting it all together, I would write the code like this:

let jsonResult = try JSONSerialization.jsonObject(with: data)

DispatchQueue.main.sync {
    if let jsonDict = jsonResult as? [String: Any],
        jsonDict["status"] as? String == "OK",
        let routes = jsonDict["routes"] as? [[String: Any]],
        let route = routes.first,
        let legs = route["legs"] as? [[String: Any]] {

        for leg in legs {
            if let duration = leg["duration_in_traffic"] as? [String: Any] {
                if let time = duration["value"] as? Int {
                    print("\(time / 60) Min")
                }
            }
        }
    }
}

Note how duration["value"] can be conditionally cast to an Int without an intermediate NSNumber.

Finally you can factor the extraction of the duration data into a separate function:

func durations(from directions: [String: Any]) -> [Int]? {
    guard directions["status"] as? String == "OK",
        let routes = directions["routes"] as? [[String: Any]],
        let route = routes.first,
        let legs = route["legs"] as? [[String: Any]]  else {
            return nil
    }

    return legs
        .flatMap { $0["duration_in_traffic"] as? [String: Any] }
        .flatMap { $0["value"] as? Int }
}

which can then be used as

if let directions = (try JSONSerialization.jsonObject(with: data)) as? [String: Any],
    let durations = durations(from: directions) {
    DispatchQueue.main.sync {
        for time in durations {
            print("\(time / 60) Min")
        }
    }
}

Here flatMap() is used to map each "leg" to its duration, this is essentially what you did with value(forKey:) in your original code.

  • 1
    Thank you so much! Your answer really improves my understanding in many ways! – Jonas Schafft Sep 17 '17 at 14:46
  • 1
    @JonasSchafft: You are welcome – I have added some more suggestions . – Martin R Sep 17 '17 at 15:08

Your Answer

 

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

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