HiveBrain v1.2.0
Get Started
← Back to all entries
patternswiftMinor

Recursive function to show directions for multiple locations

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
showlocationsfunctionrecursiveformultipledirections

Problem

I have a Mapview which shows a bunch of locations (Events). Here's the code of an Event object.

class Event: NSObject {
    var latitude: CLLocationDegrees!
    var longitude: CLLocationDegrees!

    var coordinates: CLLocationCoordinate2D {
        return CLLocationCoordinate2D(latitude: latitude, longitude: longitude)
    }

    override init() {
        super.init()
    }

    init(latitude: CLLocationDegrees, longitude: CLLocationDegrees) {
        self.latitude = latitude
        self.longitude = longitude
    }
}


I need to show directions to each of these locations and the directions should be calculated based on which location is the closest next. For example, if you start from the user's current location, it checks which location is the nearest and draws the route to that location. Then from there, it checks which is the next nearest one and so on.

I was able to accomplish this from the below code (I removed the unnecessary methods to make it shorter. I've attached a working example at the end).

```
class ViewController: UIViewController {

@IBOutlet weak var mapView: MKMapView!

private let locationManager = CLLocationManager()
private var events = [Event]()
private var coordinates = [CLLocationCoordinate2D]()

private func getDirections(#fromLocationCoord: CLLocationCoordinate2D, toLocationCoord: CLLocationCoordinate2D) {
let fromLocationMapItem = MKMapItem(placemark: MKPlacemark(coordinate: fromLocationCoord, addressDictionary: nil))
let toLocationMapItem = MKMapItem(placemark: MKPlacemark(coordinate: toLocationCoord, addressDictionary: nil))

let directionsRequest = MKDirectionsRequest()
directionsRequest.transportType = .Automobile
directionsRequest.setSource(fromLocationMapItem)
directionsRequest.setDestination(toLocationMapItem)

let directions = MKDirections(request: directionsRequest)
directions.calculateDirectionsWithCompletionHandler { (directionsRe

Solution

This probably isn't exactly what you want reviews to focus on... but my first comments must be on your Event class.

There are numerous problems here. The most egregious of which is:


What does the Event class do that the CLLocationCoordinate2D struct does not already do?

The answer is nothing.

In fact, the differences are very few. Your class unsafely uses implicitly unwrapped optionals for the latitude and longitude, while CLLocationCoordinate2D does not.

Both your class and the CLLocationCoordinate2D struct offer the same constructors. The difference is that the CLLocationCoordinate2D sets initial values for latitude and longitude for the zero-argument constructor (0, 0), allowing it to use non-optionals for latitude and longitude.

The only other difference between your class and the built-in struct is only necessary because your Event class is not a CLLocationCoordinate2D. The coordinates variables... first of all, being plural, makes me think I'm getting an array... so it should be non-plural... but it simply assumes our latitude/longitude are non-nil (which it can't safely do) and then constructs a CLLocationCoordinate2D to pass back to us (since that's what we actually need for doing map work).

And the last difference is that your class is a class where the built-in type is a struct. Did you make that decision consciously? Is there a reason you need a class versus a struct?

Now, I can easily presume that the Event class is incomplete and will eventually have other information, perhaps an event description? List of attendees? Start time & stop time? I don't know.

If that presumption is correct, then your efforts at the class so far have been decapsulating data, which we really shouldn't do.

So, here's what I'd do...

For now, we can simply create a typealias for the CLLocationCoordinate2D struct:

typealias EventLocation = CLLocationCoordinate2D


So now, we can use the CLLocationCoordinate2D, but use it with a name that matches what we're using it for. Then, if we ever write that Event class, we just add EventLocation as a property of the class (and maintain proper encapsulation).

And there's actually good reason for typealias-ing versus just using the CLLocationCoordinate2D. If you want to use CLLocationCoordinate2D, you'll have to import CoreLocation in every file you want to use it in. If you typealias it, you only have to import it in the file you declare the typealias in. So you save yourself so imports... and you keep the unnecessary parts of CoreLocation out of all the rest of your files.

Code Snippets

typealias EventLocation = CLLocationCoordinate2D

Context

StackExchange Code Review Q#98753, answer score: 4

Revisions (0)

No revisions yet.