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

Stick to chosen route #995

Merged
merged 4 commits into from
Jan 9, 2018
Merged

Stick to chosen route #995

merged 4 commits into from
Jan 9, 2018

Conversation

bsudekum
Copy link
Contributor

@bsudekum bsudekum commented Jan 9, 2018

Closes: #818

This PR seeks to stick to the route the user has selected when rerouting. This occurs by comparing the previous route summary to the new route(s) summary.

/cc @mapbox/navigation-ios @mapbox/navigation-android

@bsudekum bsudekum requested a review from 1ec5 January 9, 2018 22:31
@@ -50,4 +50,37 @@ extension String {
var containsDecimalDigit: Bool {
return self.rangeOfCharacter(from: CharacterSet.decimalDigits) != nil
}

// Adapted from https://github.com/raywenderlich/swift-algorithm-club/blob/master/Minimum%20Edit%20Distance/MinimumEditDistance.playground/Contents.swift
Copy link
Contributor

Choose a reason for hiding this comment

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

This code is substantially similar to that playground code. We’ll need to include this license either right here in this file or in LICENSE.md. That isn’t a problem at all, but here are two alternatives anyways:

  • Write the code from scratch according to the well-documented Wagner–Fischer algorithm.
  • Depend on this library instead, because it provides a Damerau–Levenshtein distance function. Unlike a basic Levenshtein distance, Damerau–Levenshtein treats a transposition (like swapping two words) as a single edit instead of a deletion and addition. We’ve had good results with this approach in a previous application.

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'd really like to steer clear of adding another dependency. I'm 👍 on adding this to a note LICENSE.md.

Copy link
Contributor

Choose a reason for hiding this comment

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

FYI, Damerau–Levenshtein is quite concise, so if adding a dependency is a problem, it shouldn’t be too difficult to port this code to Swift and add a similar MIT license block to the codebase.

@bsudekum
Copy link
Contributor Author

bsudekum commented Jan 9, 2018

Noting, this is a crutch until this feature is implemented on the Mapbox Directions API.

}
}

func selectRouteBySimilarity(from routes: [Route]) -> Route? {
Copy link
Contributor

Choose a reason for hiding this comment

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

This method returns a value without any side effects, so start the name with a noun, not a verb: mostSimilarRoute(in:).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants