Revamp waypoint types #388
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,7 +4,7 @@ class MatchResponse: Codable { | |
| var code: String | ||
| var message: String? | ||
| var matches : [Match]? | ||
| var tracepoints: [Match.Tracepoint?]? | ||
|
|
||
| private enum CodingKeys: String, CodingKey { | ||
| case code | ||
|
|
@@ -17,12 +17,14 @@ class MatchResponse: Codable { | |
| let container = try decoder.container(keyedBy: CodingKeys.self) | ||
| code = try container.decode(String.self, forKey: .code) | ||
| message = try container.decodeIfPresent(String.self, forKey: .message) | ||
| tracepoints = try container.decodeIfPresent([Match.Tracepoint?].self, forKey: .tracepoints) | ||
| matches = try container.decodeIfPresent([Match].self, forKey: .matches) | ||
|
|
||
| if let tracepoints = self.tracepoints, let matches = matches { | ||
| for match in matches { | ||
| // TODO: Filter on matchings_index. | ||
| // TODO: Push tracepoints down to individual legs to reflect waypoint_index. | ||
|
There was a problem hiding this comment. Can someone point me to some kind of a doc or explanation about There was a problem hiding this comment. Here’s the Map Matching API documentation, which is more relevant than before, now that this PR hews more closely to the Map Matching API response format. It isn’t necessary to fix #386 as part of this PR, but I think understanding that issue helped me understand these properties better. There was a problem hiding this comment. According to that doc, our There was a problem hiding this comment. The Map Matching API response format avoids repetition to minimize data transfer, and it’s optimized for a file format (JSON) that has no concept of pointer references. As a result, the response is rife with parallel structures and indices. At the same time, the Directions API response is influenced by the OSRM Router service’s heavily nested response format (maneuvers inside steps inside legs inside routes). As part of developing the “bring your own route” workflow, the Map Matching API adopted most of the Directions API’s format so that the client could duck-type matches as routes. So Map Matching API responses end up with a mix of indexable parallel structures in some places and repetitive, nested structures in other places. Originally, this library only supported the Directions API, so it adopted that API’s highly nested structure. We also avoided index-based structures, even where the Directions API had them, because of potential out-of-bounds issues and other misuse. Pointer references are more convenient to work with than indices and don’t result in excessive memory consumption. It’s also much more convenient for objects to be self-contained: if the application only cares about the match, it can pass around or archive an individual Match object instead of the entire MatchResponse plus a match index. In the long term, I think we will want to go all-in on indexed parallel structures, once a future version of the Directions API adopts Valhalla’s more consistently indexed-based structure. When we do that, things like the But in the short term, we should keep pushing things like This is a good example of how I think we want to be deliberate about where this library’s types resemble or differ from the API response. The API response format has a lot of warts, having bent over backwards to preserve backwards compatibility, so we only want to copy what make sense. We prioritized refactoring the waypoint/tracepoint types because they were too different from the corresponding API response objects, leading to some questionable patterns like treating There was a problem hiding this comment. OK, so due to some reasonable matters like API structure and JSON format we may not follow the doc strictly but instead "adjust" info we provide to users to have some adequate form. |
||
| match.tracepoints = tracepoints | ||
| } | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,40 +1,44 @@ | ||
| import Foundation | ||
| import CoreLocation | ||
|
|
||
| public extension Match { | ||
| /** | ||
| A tracepoint represents a location matched to the road network. | ||
| */ | ||
| struct Tracepoint: Matchpoint, Equatable { | ||
| // MARK: Positioning the Waypoint | ||
|
|
||
| /** | ||
| The geographic coordinate of the waypoint, snapped to the road network. | ||
| */ | ||
| public var coordinate: CLLocationCoordinate2D | ||
|
|
||
| /** | ||
| The straight-line distance from this waypoint to the corresponding waypoint in the `RouteOptions` or `MatchOptions` object. | ||
|
|
||
| The requested waypoint is snapped to the road network. This property contains the straight-line distance from the original requested waypoint’s `DirectionsOptions.Waypoint.coordinate` property to the `coordinate` property. | ||
| */ | ||
| public var correction: CLLocationDistance | ||
|
|
||
| // MARK: Determining the Degree of Confidence | ||
|
|
||
| /** | ||
| Number of probable alternative matchings for this tracepoint. A value of zero indicates that this point was matched unambiguously. | ||
| */ | ||
| public var countOfAlternatives: Int | ||
| } | ||
| } | ||
|
|
||
| extension Match.Tracepoint: Codable { | ||
| private enum CodingKeys: String, CodingKey { | ||
| case coordinate = "location" | ||
| case correction = "distance" | ||
| case countOfAlternatives = "alternatives_count" | ||
| } | ||
| } | ||
|
|
||
| extension Match.Tracepoint: CustomStringConvertible { | ||
| public var description: String { | ||
| return "<latitude: \(coordinate.latitude); longitude: \(coordinate.longitude)>" | ||
| } | ||
| } |
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.
Finish this sentence.
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.
Some context would be helpful to do it :)
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.
I think I had been going through the PR looking for breaking changes to call out but got distracted. If you see anything else that has changed in the public API besides the first item, feel free to add it here; otherwise, we can remove this item.