This repository has been archived by the owner on Aug 8, 2023. It is now read-only.
Eliminate MGLMultiPoint #12419
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
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 asMKPolygon.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
The text was updated successfully, but these errors were encountered: