Skip to content
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

Structured road names, codes, destinations #91

Merged
merged 6 commits into from
Nov 15, 2016
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 77 additions & 10 deletions MapboxDirections/MBRouteStep.swift
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,60 @@ public enum ManeuverDirection: Int, CustomStringConvertible {
}
}

extension String {
internal func tagValuesSeparatedByString(separator: String) -> [String] {
return componentsSeparatedByString(separator).map { $0.stringByTrimmingCharactersInSet(.whitespaceCharacterSet()) }.filter { !$0.isEmpty }
}
}

/**
Encapsulates all the information about a road.
*/
struct Road {
let names: [String]?
let codes: [String]?
let destinations: [String]?
let destinationCodes: [String]?

init(name: String?, ref: String?, destination: String?) {
var codes: [String]?
if let names = name, let gloss = ref ?? destination {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand the semantic meaning of gloss and the dictionary did not help me understand. The case that in Mapbox Directions v5 destination is mixed into name can not exist. Likewise it makes no sense to me that codes would ever have the contents of destination in them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Turns out Directions API v5 used to conflate destination with ref in the parenthetical, but it no longer does so, much to my relief. I’ll gladly remove the fallbacks.

// Mapbox Directions API v5 encodes the ref separately from the name but redundantly includes the ref or destination in the name for backwards compatibility. Remove the ref or destination from the name.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To clarify: This is not true, the direction is never mixed into the name, but only the ref.

let parenthetical = "(\(gloss))"
if names == gloss {
self.names = nil
} else {
self.names = names.stringByReplacingOccurrencesOfString(parenthetical, withString: "").tagValuesSeparatedByString(";")
}
codes = gloss.tagValuesSeparatedByString(";")
} else if let names = name, let codesRange = names.rangeOfString("\\(.+?\\)$", options: .RegularExpressionSearch, range: names.startIndex..<names.endIndex) {
// Mapbox Directions API v4 encodes the ref or destination inside a parenthetical. Remove the ref or destination from the name.
let parenthetical = names.substringWithRange(codesRange)
if names == ref ?? destination {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above, remove ?? destination

self.names = nil
} else {
self.names = names.stringByReplacingOccurrencesOfString(parenthetical, withString: "").tagValuesSeparatedByString(";")
}
codes = parenthetical.stringByTrimmingCharactersInSet(NSCharacterSet(charactersInString: "()")).tagValuesSeparatedByString(";")
} else {
self.names = name?.tagValuesSeparatedByString(";")
codes = nil
}

// Mapbox Directions API v5 combines the destination’s ref and name.
if let destination = destination where destination.containsString(": ") {
let destinationComponents = destination.componentsSeparatedByString(": ")
self.destinationCodes = destinationComponents.first?.tagValuesSeparatedByString(",")
self.destinations = destinationComponents.dropFirst().joinWithSeparator(": ").tagValuesSeparatedByString(",")
} else {
self.destinationCodes = nil
self.destinations = destination?.tagValuesSeparatedByString(",")
}

self.codes = codes
}
}

/**
A `RouteStep` object represents a single distinct maneuver along a route and the approach to the next maneuver. The route step object corresponds to a single instruction the user must follow to complete a portion of the route. For example, a step might require the user to turn then follow a road.

Expand All @@ -382,7 +436,11 @@ public class RouteStep: NSObject, NSSecureCoding {

internal init(finalHeading: CLLocationDirection?, maneuverType: ManeuverType?, maneuverDirection: ManeuverDirection?, maneuverLocation: CLLocationCoordinate2D, name: String?, coordinates: [CLLocationCoordinate2D]?, json: JSONDictionary) {
transportType = TransportType(description: json["mode"] as! String)
destinations = json["destinations"] as? String

let road = Road(name: name, ref: json["ref"] as? String, destination: json["destinations"] as? String)
names = road.names
codes = road.codes ?? road.destinationCodes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are destinationCodes mixed into codes here? wouldn't it make more sense to create a new property to store them instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea here was to make it easier for applications to display a route shield for an upcoming ramp based on codes without having to consult a separate property. We’ve since moved away from the notion that MapboxDirections.swift should perform any sort of munging, so I’ll remove this fallback.

I’ve gone back and forth quite a bit about whether destination codes should be conflated with destination or live in a separate property. In OSM, destination:ref usually follows a different format and encodes more information than ref. For example, if I-80 travels over a ramp to go from one motorway to another, the highway=motorway_link way is tagged ref=I 80 but may also be tagged destination:ref=I-80 West. The format is loose enough that I don’t feel confident making destinationCodes look like codes. But I guess it can’t hurt much.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. destinationCodes is a separate property now.

destinations = road.destinations
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume these are properties on self.? Either these should be set that way, or below assignments should not have self.= either (like self.intersections = -> intersections =)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’ll remove self. from self.intersections, since there isn’t a conflict with a local variable named intersections.


let maneuver = json["maneuver"] as! JSONDictionary
instructions = maneuver["instruction"] as! String
Expand All @@ -399,8 +457,6 @@ public class RouteStep: NSObject, NSSecureCoding {
self.maneuverDirection = maneuverDirection
exitIndex = maneuver["exit"] as? Int

self.name = name

self.maneuverLocation = maneuverLocation
self.coordinates = coordinates
}
Expand Down Expand Up @@ -471,14 +527,15 @@ public class RouteStep: NSObject, NSSecureCoding {
exitIndex = decoder.containsValueForKey("exitIndex") ? decoder.decodeIntegerForKey("exitIndex") : nil
distance = decoder.decodeDoubleForKey("distance")
expectedTravelTime = decoder.decodeDoubleForKey("expectedTravelTime")
name = decoder.decodeObjectForKey("name") as? String
names = decoder.decodeObjectOfClasses([NSArray.self, NSString.self], forKey: "names") as? [String]

guard let transportTypeDescription = decoder.decodeObjectOfClass(NSString.self, forKey: "transportType") as? String else {
return nil
}
transportType = TransportType(description: transportTypeDescription)

destinations = decoder.decodeObjectOfClass(NSString.self, forKey: "destinations") as? String
codes = decoder.decodeObjectOfClasses([NSArray.self, NSString.self], forKey: "codes") as? [String]
destinations = decoder.decodeObjectOfClasses([NSArray.self, NSString.self], forKey: "destinations") as? [String]

intersections = decoder.decodeObjectOfClasses([NSArray.self, Intersection.self], forKey: "intersections") as? [Intersection]
}
Expand Down Expand Up @@ -519,8 +576,9 @@ public class RouteStep: NSObject, NSSecureCoding {

coder.encodeDouble(distance, forKey: "distance")
coder.encodeDouble(expectedTravelTime, forKey: "expectedTravelTime")
coder.encodeObject(name, forKey: "name")
coder.encodeObject(names, forKey: "names")
coder.encodeObject(transportType?.description, forKey: "transportType")
coder.encodeObject(codes, forKey: "codes")
coder.encodeObject(destinations, forKey: "destinations")
}

Expand Down Expand Up @@ -637,11 +695,20 @@ public class RouteStep: NSObject, NSSecureCoding {
public let expectedTravelTime: NSTimeInterval

/**
The name of the road or path leading from this step’s maneuver to the next step’s maneuver.
The names of the road or path leading from this step’s maneuver to the next step’s maneuver.

If the maneuver is a turning maneuver, the step’s name is the name of the road or path onto which the user turns. If you display the name to the user, you may need to abbreviate common words like “East” or “Boulevard” to ensure that it fits in the allotted space.
*/
public let names: [String]?

/**
Any route reference codes assigned to the road or path leading from this step’s maneuver to the next step’s maneuver.

A route reference code commonly consists of an alphabetic network code, a space, and a route number. You should not assume that the network code is globally unique: for example, a network code of “NH” may indicate a “National Highway” or “New Hampshire”. Moreover, a route number may not even uniqely identify a route within a given network.

If the maneuver is a turning maneuver, the step’s name is the name of the road or path onto which the user turns. The name includes any route designations assigned to the road. If you display the name to the user, you may need to abbreviate common words like “East” or “Boulevard” to ensure that it fits in the allotted space.
If a highway off-ramp is part of a numbered route, its reference code takes precedence over any reference code associated with the ramp’s destinations.
*/
public let name: String?
public let codes: [String]?

// MARK: Getting Additional Step Details

Expand All @@ -657,7 +724,7 @@ public class RouteStep: NSObject, NSSecureCoding {

This property is typically available in steps leading to or from a freeway or expressway.
*/
public let destinations: String?
public let destinations: [String]?

/**
An array of intersections along the step.
Expand Down

Large diffs are not rendered by default.

Large diffs are not rendered by default.

3 changes: 2 additions & 1 deletion MapboxDirectionsTests/V4Tests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ class V4Tests: XCTestCase {
XCTAssertEqual(step.distance, 223582.0)
XCTAssertEqual(step.expectedTravelTime, 7219.0)
XCTAssertEqual(step.instructions, "Go straight onto I 80;US 93 Alternate, I 80;US 93 ALT becomes I 80;US 93 Alternate")
XCTAssertEqual(step.name, "I 80;US 93 Alternate")
XCTAssertNotNil(step.names)
XCTAssertEqual(step.names ?? [], ["I 80", "US 93 Alternate"])
XCTAssertEqual(step.maneuverType, ManeuverType.Continue)
XCTAssertNil(step.maneuverDirection)
XCTAssertNil(step.initialHeading)
Expand Down
57 changes: 32 additions & 25 deletions MapboxDirectionsTests/V5Tests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class V5Tests: XCTestCase {
XCTAssertNil(error, "Error: \(error)")

XCTAssertNotNil(routes)
XCTAssertEqual(routes!.count, 1)
XCTAssertEqual(routes!.count, 2)
route = routes!.first!

expectation.fulfill()
Expand All @@ -54,7 +54,7 @@ class V5Tests: XCTestCase {

XCTAssertNotNil(route)
XCTAssertNotNil(route!.coordinates)
XCTAssertEqual(route!.coordinates!.count, 842)
XCTAssertEqual(route!.coordinates!.count, 28_442)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the semantics of 28_442? Is this a range or math exponent syntax?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_ groups digits; it’s for human edification only and has no effect on the literal’s meaning. (C++ uses ' for the same purpose.)


// confirming actual decoded values is important because the Directions API
// uses an atypical precision level for polyline encoding
Expand All @@ -63,40 +63,47 @@ class V5Tests: XCTestCase {
XCTAssertEqual(route!.legs.count, 1)

let leg = route!.legs.first!
XCTAssertEqual(leg.name, "CA 24, Camino Tassajara")
XCTAssertEqual(leg.steps.count, 22)
XCTAssertEqual(leg.name, "I 80, I 80;US 30")
XCTAssertEqual(leg.steps.count, 59)

let step = leg.steps[16]
XCTAssertEqual(round(step.distance), 166)
XCTAssertEqual(round(step.expectedTravelTime), 13)
XCTAssertEqual(step.instructions, "Take the ramp on the right")
let step = leg.steps[43]
XCTAssertEqual(round(step.distance), 688)
XCTAssertEqual(round(step.expectedTravelTime), 30)
XCTAssertEqual(step.instructions, "Take the ramp on the right towards Washington")

XCTAssertEqual(step.name, "")
XCTAssertEqual(step.destinations, "Sycamore Valley Road")
XCTAssertNil(step.names)
XCTAssertNotNil(step.destinations)
XCTAssertEqual(step.destinations ?? [], ["Washington"])
XCTAssertEqual(step.maneuverType, ManeuverType.TakeOffRamp)
XCTAssertEqual(step.maneuverDirection, ManeuverDirection.SlightRight)
XCTAssertEqual(step.initialHeading, 182)
XCTAssertEqual(step.finalHeading, 196)
XCTAssertEqual(step.initialHeading, 90)
XCTAssertEqual(step.finalHeading, 96)

XCTAssertNotNil(step.coordinates)
XCTAssertEqual(step.coordinates!.count, 5)
XCTAssertEqual(step.coordinates!.count, 17)
XCTAssertEqual(step.coordinates!.count, Int(step.coordinateCount))
let coordinate = step.coordinates!.first!
XCTAssertEqual(round(coordinate.latitude), 38)
XCTAssertEqual(round(coordinate.longitude), -122)
XCTAssertEqual(round(coordinate.latitude), 39)
XCTAssertEqual(round(coordinate.longitude), -77)

XCTAssertEqual(leg.steps[18].name, "Sycamore Valley Road West")
XCTAssertNil(leg.steps[28].names)
XCTAssertEqual(leg.steps[28].codes ?? [], ["I 80"])
XCTAssertEqual(leg.steps[28].destinations ?? [], ["Toll Road"])

let intersection = step.intersections!.first!
XCTAssertEqual(intersection.outletIndexes, NSIndexSet(indexesInRange: NSRange(location: 1, length: 2)))
XCTAssertEqual(intersection.approachIndex, 0)
XCTAssertEqual(intersection.outletIndex, 2)
XCTAssertEqual(intersection.headings, [0, 180, 195])
XCTAssertNotNil(intersection.location.latitude)
XCTAssertNotNil(intersection.location.longitude)
XCTAssertEqual(intersection.usableApproachLanes, NSIndexSet(indexesInRange: NSRange(location: 0, length: 2)))
let intersections = leg.steps[40].intersections
XCTAssertNotNil(intersections)
XCTAssertEqual(intersections?.count, 7)
let intersection = intersections?[2]
XCTAssertEqual(intersection?.outletIndexes.containsIndex(0), true)
XCTAssertEqual(intersection?.outletIndexes.containsIndexesInRange(NSRange(location: 2, length: 2)), true)
XCTAssertEqual(intersection?.approachIndex, 1)
XCTAssertEqual(intersection?.outletIndex, 3)
XCTAssertEqual(intersection?.headings ?? [], [15, 90, 195, 270])
XCTAssertNotNil(intersection?.location.latitude)
XCTAssertNotNil(intersection?.location.longitude)
XCTAssertEqual(intersection?.usableApproachLanes ?? [], NSIndexSet(indexesInRange: NSRange(location: 1, length: 3)))

let lane = intersection.approachLanes?.first
let lane = intersection?.approachLanes?.first
let indications = lane?.indications
XCTAssertNotNil(indications)
XCTAssertTrue(indications!.contains(.Left))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it might make sense to add a test for step.names being populated.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a test for this case.

Expand Down
4 changes: 2 additions & 2 deletions RouteStepTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ class RouteStepTests: XCTestCase {
XCTAssertEqual(unarchivedStep.exitIndex, step.exitIndex)
XCTAssertEqual(unarchivedStep.distance, step.distance)
XCTAssertEqual(unarchivedStep.expectedTravelTime, step.expectedTravelTime)
XCTAssertEqual(unarchivedStep.name, step.name)
XCTAssertEqual(unarchivedStep.names ?? [], step.names ?? [])
XCTAssertEqual(unarchivedStep.transportType, step.transportType)
XCTAssertEqual(unarchivedStep.destinations, step.destinations)
XCTAssertEqual(unarchivedStep.destinations ?? [], step.destinations ?? [])
}
}