Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Eliminate MGLMultiPoint #12419

Open
1ec5 opened this issue Jul 18, 2018 · 0 comments
Open

Eliminate MGLMultiPoint #12419

1ec5 opened this issue Jul 18, 2018 · 0 comments
Labels
gl-ios iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS refactor SEMVER-MAJOR Requires a major release according to Semantic Versioning rules

Comments

@1ec5
Copy link
Contributor

1ec5 commented Jul 18, 2018

We should remove the MGLMultiPoint abstract class and make MGLPolyline and MGLPolygon inherit directly from MGLShape.

MGLMultiPoint exists for historical reasons (modeled after MapKit’s MKMultiPoint) and in order for MGLPolyline and MGLPolygon to share most of their implementation. With the introduction of additional Simple Features classes like MGLMultiPolyline and MGLPolylineFeature in #5110, we’ve moved pretty far from hewing to MapKit’s annotation object model. (For example, MGLPolygon.interiorPolygons doesn’t have the same semantics as MKPolygon.interiorPolygons.) Meanwhile, most of the code is duplicated within MGLMultiPolyline and MGLMultiPolygon anyways.

In the public API, MGLMultiPoint appears only twice, in order for MGLPolyline and MGLPolygon to inherit from it. Thus important members such as coordinates suffer from poor discoverability, compounded by the usual awkwardness of an abstract class in Objective-C, as exemplified by the documentation comment’s exhortation not to initialize or subclass an MGLMultiPoint directly.

The method implementations in MGLMultiPoint will need to be copied into both MGLPolyline and MGLPolygon. If code duplication turns out to be problematic, then we should explore ways to share code among MGLPolyline, MGLPolygon, MGLMultiPolyline and MGLMultiPolygon that don’t involve an abstract class. The private MGLMultiPointDelegate protocol should be renamed to MGLOverlayDelegate, reflecting the fact that MGLOverlay conformance more or less determines whether a class can be used as an annotation.

This would be a backwards-incompatible change, if only because applications may be using MGLMultiPoint as shorthand for “either MGLPolyline or MGLPolygon”. Ideally #7454 would be undertaken in the same release cycle.

/ref #3288 #5201 (comment)
/cc @captainbarbosa @fabian-guerra @jfirebaugh

@1ec5 1ec5 added iOS Mapbox Maps SDK for iOS refactor macOS Mapbox Maps SDK for macOS SEMVER-MAJOR Requires a major release according to Semantic Versioning rules labels Jul 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
gl-ios iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS refactor SEMVER-MAJOR Requires a major release according to Semantic Versioning rules
Projects
None yet
Development

No branches or pull requests

2 participants