patternswiftMinor
Recursive function to show directions for multiple locations
Viewed 0 times
showlocationsfunctionrecursiveformultipledirections
Problem
I have a Mapview which shows a bunch of locations (
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
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
There are numerous problems here. The most egregious of which is:
What does the
The answer is nothing.
In fact, the differences are very few. Your class unsafely uses implicitly unwrapped optionals for the latitude and longitude, while
Both your class and the
The only other difference between your class and the built-in struct is only necessary because your
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
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
So now, we can use the
And there's actually good reason for
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 = CLLocationCoordinate2DSo 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 = CLLocationCoordinate2DContext
StackExchange Code Review Q#98753, answer score: 4
Revisions (0)
No revisions yet.