Skip to content

Comments

Change route on the fly#302

Merged
frederoni merged 3 commits intomasterfrom
fred-change-route
Jun 22, 2017
Merged

Change route on the fly#302
frederoni merged 3 commits intomasterfrom
fred-change-route

Conversation

@frederoni
Copy link
Contributor

@frederoni frederoni commented Jun 21, 2017

This PR makes it a bit more convenient to change the route on the fly by notifying the RouteMapViewController and the RouteTableViewController about the update along with initializing a RouteProgress for the new route.

@bsudekum @ericrwolfe 👀

@frederoni frederoni requested review from 1ec5 and bsudekum June 21, 2017 13:27

if route != routeController.routeProgress.route {
routeController.routeProgress = RouteProgress(route: route)
routeController.routeProgress.currentLegProgress.stepIndex = 0
Copy link
Contributor

Choose a reason for hiding this comment

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

This is set to 0 when reseting the route progress.

}
}

if route != routeController.routeProgress.route {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this check safe when we don't implement -hash, -isEqual: or an overloaded operator?

Copy link
Contributor

@bsudekum bsudekum Jun 21, 2017

Choose a reason for hiding this comment

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

Do we actually even need to check if theyre different? Couldn't we just always blow it away

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can, but it should then be moved to the route’s didSet to avoid initializing an excessive RouteProgress.

Copy link
Contributor

Choose a reason for hiding this comment

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

If we don't override isEqual, it ends up doing a pointer equality check. That won't be as thorough.

}
}

class func downloadRouteFixture(coordinates: [CLLocationCoordinate2D], fileName: String, completion: @escaping () -> Void) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We could put this behind an #if to prevent ourselves from checking in tests that call this function.

@frederoni frederoni merged commit ff7311d into master Jun 22, 2017
@frederoni frederoni deleted the fred-change-route branch June 22, 2017 09:31
@1ec5 1ec5 added this to the v0.5.0 milestone Jul 9, 2017
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.

3 participants