-
Notifications
You must be signed in to change notification settings - Fork 90
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Provide convenience initializers for options #685
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -98,6 +98,16 @@ open class RouteOptions: DirectionsOptions { | |
self.arriveBy = arriveBy | ||
} | ||
} | ||
|
||
/** | ||
Initializes a route options object for routes between the given waypoints and an optional profile identifier. | ||
|
||
- parameter waypoints: An array of `Waypoint` objects representing locations that the route should visit in chronological order. The array should contain at least two waypoints (the source and destination) and at most 25 waypoints. (Some profiles, such as `ProfileIdentifier.automobileAvoidingTraffic`, [may have lower limits](https://www.mapbox.com/api-documentation/#directions).) | ||
- parameter profileIdentifier: A string specifying the primary mode of transportation for the routes. `ProfileIdentifier.automobile` is used by default. | ||
*/ | ||
public convenience init(waypoints: [Waypoint], profileIdentifier: ProfileIdentifier? = nil) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let’s add a test that subclasses RouteOptions and overrides just this initializer, not There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it sufficient to subclass |
||
self.init(waypoints: waypoints, profileIdentifier: profileIdentifier, queryItems: nil) | ||
} | ||
|
||
#if canImport(CoreLocation) | ||
/** | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -363,3 +363,11 @@ var testRouteOptions: RouteOptions { | |
|
||
return opts | ||
} | ||
|
||
var routeOptionsSubclassTest = testRouteOptionsSubclass(waypoints: []) | ||
|
||
class testRouteOptionsSubclass: RouteOptions { | ||
convenience init(waypoints: [Waypoint], profileIdentifier: ProfileIdentifier?) { | ||
self.init(waypoints: [], profileIdentifier: nil, queryItems: nil) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you try to match what NavigationRouteOptions was doing here more closely, so we can be sure the incompatibility is fixed? Alternatively, you could try checking out navigation SDK v2.3.x and pointing it to this branch to see if it’ll compile. Thanks! |
||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since developers should not call
DirectionsOptions(waypoints:profileIdentifier:)
directly, is it safe to not include this change?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should include it, because it is valid, if somewhat contrived, to subclass DirectionsOptions directly. The use case would be building support for another navigation API service endpoint. Normally that’s something this library would have built right in, but in principle, client code could implement that support all the same.