mapbox / mapbox/mapbox-directions-swift

RouteOptions and Waypoint should be structs

Open
#564 1 comment 1 reaction 0 assignees View on GitHub
backwards incompatible jira-sync-complete op-ex
Dominant language
Swift
Stars
206
Forks
99
Avg merge
7h 54m
Merged PRs (30d)
3

Description

RouteOptions, MatchOptions, Waypoint, and Tracepoint should all be structs, not classes. We pass these types around as if they’re grab bags of values; ideally, they would have no identity apart from their values.

## Background

This library has always represented the set of options for an API request as an object, even though a struct would be a more correct representation of pass-by-value semantics. We kept RouteOptions as a class because the library had to be compatible with Objective-C code; Swift structs don’t bridge to Objective-C. From there, we started using inheritance to add flexibility to the request functionality. To support the Map Matching API, #236 introduced DirectionsOptions as an abstract base class for RouteOptions and MatchOptions. Meanwhile, mapbox/mapbox-navigation-ios#531 added a subclass of RouteOptions, NavigationRouteOptions, to provide good default values optimized for turn-by-turn navigation.

## Implementation notes

Dropping Objective-C compatibility in #382 gives us an opportunity to convert RouteOptions and MatchOptions into structs. It’s more complicated than that, because waypoints also had to be converted to structs at the same time. It turns out that the Directions API has two different kinds of waypoints, with disjoint sets of information: one that goes in requests and another that goes in responses.

NavigationRouteOptions does nothing more than set some default values in its convenience initializer. We can safely replace it with an extension method on RouteOptions.

We allow and sometimes encourage developers to subclass NavigationRouteOptions to fine-tune the Directions API requests that come from the application. For example, mapbox/mapbox-navigation-ios-examples#91 illustrates how to use beta query parameters that the MapboxDirections library doesn’t formally support yet. Structs can’t be subclassed in Swift, so we need a different solution for customizations that MapboxDirections doesn’t or can’t support yet.

## Next steps

#388 replaces the Waypoint class with separate structs for request and response waypoints, with a goal of eventually converting RouteOptions to a struct as well. This PR has the potential to complicate client code, particularly in the navigation SDK, because client code now has to distinguish between request and response waypoints and potentially convert between the two at various times. Unfortunately, we were unable to prioritize landing this PR for a long time due to unrelated constraints, but the use of classes for input types remains an impediment to adopting more server API features and fully supporting client customization.

/cc @mapbox/navigation-ios

Contributor guide

Open the contributing guide

Research direction

Start by reviewing RouteOptions, MatchOptions, Waypoint, Tracepoint, and NavigationRouteOptions, then read the context from PR #388. Map the request and response waypoint requirements and the existing customization use cases before determining the replacement API. Done means these option and waypoint types use value semantics without losing the described navigation customization support.

Written by the indexing model from the issue text.

Assessment

Tech stack
swift
Domain
api
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.