[ios, osx] Streamlined conversion to MGLFeature

Made writability of MGLFeature properties private, to avoid having to awkwardly explain that changes to the properties don’t change the map (maybe someday they will). This allows the identifier and attributes to be set in only one place, avoiding copy-pasta.

3 files changed, 150 insertions, 145 deletions

diff --git a/platform/darwin/src/MGLFeature.h b/platform/darwin/src/MGLFeature.h
index 389b8ab67f..b593ff70e4 100644
--- a/platform/darwin/src/MGLFeature.h
+++ b/platform/darwin/src/MGLFeature.h
@@ -89,10 +89,6 @@ NS_ASSUME_NONNULL_BEGIN
  <a href="https://www.mapbox.com/mapbox-gl-style-spec/#sources">tile source</a>.
  */
 @interface MGLPointFeature : MGLPointAnnotation <MGLFeature>
-
-@property (nonatomic, copy, nullable, readwrite) id identifier;
-@property (nonatomic, copy, readwrite) NS_DICTIONARY_OF(NSString *, id) *attributes;
-
 @end
 
 /**
@@ -100,10 +96,6 @@ NS_ASSUME_NONNULL_BEGIN
  <a href="https://www.mapbox.com/mapbox-gl-style-spec/#sources">tile source</a>.
  */
 @interface MGLPolylineFeature : MGLPolyline <MGLFeature>
-
-@property (nonatomic, copy, nullable, readwrite) id identifier;
-@property (nonatomic, copy, readwrite) NS_DICTIONARY_OF(NSString *, id) *attributes;
-
 @end
 
 /**
@@ -111,10 +103,6 @@ NS_ASSUME_NONNULL_BEGIN
  <a href="https://www.mapbox.com/mapbox-gl-style-spec/#sources">tile source</a>.
  */
 @interface MGLPolygonFeature : MGLPolygon <MGLFeature>
-
-@property (nonatomic, copy, nullable, readwrite) id identifier;
-@property (nonatomic, copy, readwrite) NS_DICTIONARY_OF(NSString *, id) *attributes;
-
 @end
 
 /**
@@ -122,10 +110,6 @@ NS_ASSUME_NONNULL_BEGIN
  <a href="https://www.mapbox.com/mapbox-gl-style-spec/#sources">tile source</a>.
  */
 @interface MGLMultiPointFeature : MGLMultiPoint <MGLFeature>
-
-@property (nonatomic, copy, nullable, readwrite) id identifier;
-@property (nonatomic, copy, readwrite) NS_DICTIONARY_OF(NSString *, id) *attributes;
-
 @end
 
 /**
@@ -133,10 +117,6 @@ NS_ASSUME_NONNULL_BEGIN
  <a href="https://www.mapbox.com/mapbox-gl-style-spec/#sources">tile source</a>.
  */
 @interface MGLMultiPolylineFeature : MGLMultiPolyline <MGLFeature>
-
-@property (nonatomic, copy, nullable, readwrite) id identifier;
-@property (nonatomic, copy, readwrite) NS_DICTIONARY_OF(NSString *, id) *attributes;
-
 @end
 
 /**
@@ -144,10 +124,6 @@ NS_ASSUME_NONNULL_BEGIN
  <a href="https://www.mapbox.com/mapbox-gl-style-spec/#sources">tile source</a>.
  */
 @interface MGLMultiPolygonFeature : MGLMultiPolygon <MGLFeature>
-
-@property (nonatomic, copy, nullable, readwrite) id identifier;
-@property (nonatomic, copy, readwrite) NS_DICTIONARY_OF(NSString *, id) *attributes;
-
 @end
 
 /**
@@ -155,10 +131,6 @@ NS_ASSUME_NONNULL_BEGIN
  <a href="https://www.mapbox.com/mapbox-gl-style-spec/#sources">tile source</a>.
  */
 @interface MGLShapeCollectionFeature : MGLShapeCollection <MGLFeature>
-
-@property (nonatomic, copy, nullable, readwrite) id identifier;
-@property (nonatomic, copy, readwrite) NS_DICTIONARY_OF(NSString *, id) *attributes;
-
 @end
 
 NS_ASSUME_NONNULL_END
diff --git a/platform/darwin/src/MGLFeature.mm b/platform/darwin/src/MGLFeature.mm
index 68c57a193a..3e60b2061b 100644
--- a/platform/darwin/src/MGLFeature.mm
+++ b/platform/darwin/src/MGLFeature.mm
@@ -6,10 +6,116 @@
 
 #import "MGLMultiPoint_Private.h"
 
+@protocol MGLFeaturePrivate <MGLFeature>
+
+@property (nonatomic, copy, nullable, readwrite) id identifier;
+@property (nonatomic, copy, readwrite) NS_DICTIONARY_OF(NSString *, id) *attributes;
+
+@end
+
+@interface MGLPointFeature () <MGLFeaturePrivate>
+@end
+
+@implementation MGLPointFeature
+
+@synthesize identifier;
+@synthesize attributes;
+
+- (id)attributeForKey:(NSString *)key {
+    return self.attributes[key];
+}
+
+@end
+
+@interface MGLPolylineFeature () <MGLFeaturePrivate>
+@end
+
+@implementation MGLPolylineFeature
+
+@synthesize identifier;
+@synthesize attributes;
+
+- (id)attributeForKey:(NSString *)key {
+    return self.attributes[key];
+}
+
+@end
+
+@interface MGLPolygonFeature () <MGLFeaturePrivate>
+@end
+
+@implementation MGLPolygonFeature
+
+@synthesize identifier;
+@synthesize attributes;
+
+- (id)attributeForKey:(NSString *)key {
+    return self.attributes[key];
+}
+
+@end
+
+@interface MGLMultiPointFeature () <MGLFeaturePrivate>
+@end
+
+@implementation MGLMultiPointFeature
+
+@synthesize identifier;
+@synthesize attributes;
+
+- (id)attributeForKey:(NSString *)key {
+    return self.attributes[key];
+}
+
+@end
+
+@interface MGLMultiPolylineFeature () <MGLFeaturePrivate>
+@end
+
+@implementation MGLMultiPolylineFeature
+
+@synthesize identifier;
+@synthesize attributes;
+
+- (id)attributeForKey:(NSString *)key {
+    return self.attributes[key];
+}
+
+@end
+
+@interface MGLMultiPolygonFeature () <MGLFeaturePrivate>
+@end
+
+@implementation MGLMultiPolygonFeature
+
+@synthesize identifier;
+@synthesize attributes;
+
+- (id)attributeForKey:(NSString *)key {
+    return self.attributes[key];
+}
+
+@end
+
+@interface MGLShapeCollectionFeature () <MGLFeaturePrivate>
+@end
+
+@implementation MGLShapeCollectionFeature
+
+@synthesize identifier;
+@synthesize attributes;
+
+- (id)attributeForKey:(NSString *)key {
+    return self.attributes[key];
+}
+
+@end
+
+/**
+ Recursively transforms a C++ type into the corresponding Foundation type.
+ */
 class PropertyValueEvaluator {
 public:
-    PropertyValueEvaluator() {}
-    
     id operator()(const std::nullptr_t &) const {
         return [NSNull null];
     }
@@ -37,10 +143,9 @@ public:
     id operator()(const std::vector<mbgl::Value> &values) const {
         std::vector<id> objects;
         objects.reserve(values.size());
-        std::transform(values.begin(), values.end(), std::back_inserter(objects), ^id (const mbgl::Value &value) {
-            PropertyValueEvaluator evaluator;
-            return mbgl::Value::visit(value, evaluator);
-        });
+        for (const auto &v : values) {
+            objects.push_back(mbgl::Value::visit(v, *this));
+        }
         return [NSArray arrayWithObjects:&objects[0] count:objects.size()];
     }
     
@@ -51,49 +156,34 @@ public:
         objects.reserve(items.size());
         for (auto &item : items) {
             keys.push_back(@(item.first.c_str()));
-            PropertyValueEvaluator evaluator;
-            objects.push_back(mbgl::Value::visit(item.second, evaluator));
+            objects.push_back(mbgl::Value::visit(item.second, *this));
         }
         return [NSDictionary dictionaryWithObjects:&objects[0] forKeys:&keys[0] count:keys.size()];
     }
 };
 
+/**
+ Transforms an `mbgl::geometry::geometry` type into an instance of the
+ corresponding Objective-C geometry class.
+ */
 template <typename T>
 class GeometryEvaluator {
 public:
-    GeometryEvaluator()
-    : attributes([NSMutableDictionary dictionary]) {}
-    
-    GeometryEvaluator(const mbgl::Feature &feature)
-    : tag(feature.id ? @(*feature.id) : nullptr),
-    attributes([NSMutableDictionary dictionaryWithCapacity:feature.properties.size()]) {
-        for (auto &pair : feature.properties) {
-            auto &value = pair.second;
-            PropertyValueEvaluator evaluator;
-            attributes[@(pair.first.c_str())] = mbgl::Value::visit(value, evaluator);
-        }
-    }
-    
-    MGLShape <MGLFeature> * operator()(const mapbox::geometry::point<T> &geometry) const {
+    MGLShape <MGLFeaturePrivate> * operator()(const mapbox::geometry::point<T> &geometry) const {
         MGLPointFeature *feature = [[MGLPointFeature alloc] init];
         feature.coordinate = coordinateFromPoint(geometry);
-        feature.identifier = tag;
-        feature.attributes = attributes;
         return feature;
     }
     
-    MGLShape <MGLFeature> * operator()(const mapbox::geometry::line_string<T> &geometry) const {
+    MGLShape <MGLFeaturePrivate> * operator()(const mapbox::geometry::line_string<T> &geometry) const {
         std::vector<CLLocationCoordinate2D> coordinates;
         coordinates.reserve(geometry.size());
         std::transform(geometry.begin(), geometry.end(), std::back_inserter(coordinates), coordinateFromPoint);
         
-        MGLPolylineFeature *feature = [[MGLPolylineFeature alloc] initWithCoordinates:&coordinates[0] count:coordinates.size()];
-        feature.identifier = tag;
-        feature.attributes = attributes;
-        return feature;
+        return [[MGLPolylineFeature alloc] initWithCoordinates:&coordinates[0] count:coordinates.size()];
     }
     
-    MGLShape <MGLFeature> * operator()(const mapbox::geometry::polygon<T> &geometry) const {
+    MGLShape <MGLFeaturePrivate> * operator()(const mapbox::geometry::polygon<T> &geometry) const {
         // TODO: MGLPolygon doesn’t support holes, so what to do?
         auto &linearRing = geometry.front();
         
@@ -101,24 +191,18 @@ public:
         coordinates.reserve(linearRing.size());
         std::transform(linearRing.begin(), linearRing.end(), std::back_inserter(coordinates), coordinateFromPoint);
         
-        MGLPolygonFeature *feature = [[MGLPolygonFeature alloc] initWithCoordinates:&coordinates[0] count:coordinates.size()];
-        feature.identifier = tag;
-        feature.attributes = attributes;
-        return feature;
+        return [[MGLPolygonFeature alloc] initWithCoordinates:&coordinates[0] count:coordinates.size()];
     }
     
-    MGLShape <MGLFeature> * operator()(const mapbox::geometry::multi_point<T> &geometry) const {
+    MGLShape <MGLFeaturePrivate> * operator()(const mapbox::geometry::multi_point<T> &geometry) const {
         std::vector<CLLocationCoordinate2D> coordinates;
         coordinates.reserve(geometry.size());
         std::transform(geometry.begin(), geometry.end(), std::back_inserter(coordinates), coordinateFromPoint);
         
-        MGLMultiPointFeature *feature = [[MGLMultiPointFeature alloc] initWithCoordinates:&coordinates[0] count:coordinates.size()];
-        feature.identifier = tag;
-        feature.attributes = attributes;
-        return feature;
+        return [[MGLMultiPointFeature alloc] initWithCoordinates:&coordinates[0] count:coordinates.size()];
     }
     
-    MGLShape <MGLFeature> * operator()(const mapbox::geometry::multi_line_string<T> &geometry) const {
+    MGLShape <MGLFeaturePrivate> * operator()(const mapbox::geometry::multi_line_string<T> &geometry) const {
         NSMutableArray *polylines = [NSMutableArray arrayWithCapacity:geometry.size()];
         for (auto &lineString : geometry) {
             std::vector<CLLocationCoordinate2D> coordinates;
@@ -129,13 +213,10 @@ public:
             [polylines addObject:polyline];
         }
         
-        MGLMultiPolylineFeature *feature = [MGLMultiPolylineFeature multiPolylineWithPolylines:polylines];
-        feature.identifier = tag;
-        feature.attributes = attributes;
-        return feature;
+        return [MGLMultiPolylineFeature multiPolylineWithPolylines:polylines];
     }
     
-    MGLShape <MGLFeature> * operator()(const mapbox::geometry::multi_polygon<T> &geometry) const {
+    MGLShape <MGLFeaturePrivate> * operator()(const mapbox::geometry::multi_polygon<T> &geometry) const {
         NSMutableArray *polygons = [NSMutableArray arrayWithCapacity:geometry.size()];
         for (auto &polygon : geometry) {
             // TODO: MGLPolygon doesn’t support holes, so what to do?
@@ -149,29 +230,21 @@ public:
             [polygons addObject:polygonObject];
         }
         
-        MGLMultiPolygonFeature *feature = [MGLMultiPolygonFeature multiPolygonWithPolygons:polygons];
-        feature.identifier = tag;
-        feature.attributes = attributes;
-        return feature;
+        return [MGLMultiPolygonFeature multiPolygonWithPolygons:polygons];
     }
     
-    MGLShape <MGLFeature> * operator()(const mapbox::geometry::geometry_collection<T> &collection) const {
+    MGLShape <MGLFeaturePrivate> * operator()(const mapbox::geometry::geometry_collection<T> &collection) const {
         NSMutableArray *shapes = [NSMutableArray arrayWithCapacity:collection.size()];
         for (auto &geometry : collection) {
+            // This is very much like the transformation that happens in MGLFeaturesFromMBGLFeatures(), but these are raw geometries with no associated feature IDs or attributes.
             GeometryEvaluator<T> evaluator;
-            id <MGLFeature> feature = mapbox::geometry::geometry<T>::visit(geometry, evaluator);
-            [shapes addObject:feature];
+            MGLShape <MGLFeaturePrivate> *shape = mapbox::geometry::geometry<T>::visit(geometry, evaluator);
+            [shapes addObject:shape];
         }
-        MGLShapeCollectionFeature *feature = [MGLShapeCollectionFeature shapeCollectionWithShapes:shapes];
-        feature.identifier = tag;
-        feature.attributes = attributes;
-        return feature;
+        return [MGLShapeCollectionFeature shapeCollectionWithShapes:shapes];
     }
     
 private:
-    NSNumber *tag = nullptr;
-    NS_MUTABLE_DICTIONARY_OF(NSString *, id) *attributes;
-    
     static CLLocationCoordinate2D coordinateFromPoint(const mapbox::geometry::point<T> &point) {
         return CLLocationCoordinate2DMake(point.y, point.x);
     }
@@ -181,64 +254,20 @@ NS_ARRAY_OF(MGLShape <MGLFeature> *) *MGLFeaturesFromMBGLFeatures(const std::vec
     std::vector<MGLShape <MGLFeature> *> shapes;
     shapes.reserve(features.size());
     std::transform(features.begin(), features.end(), std::back_inserter(shapes), ^MGLShape <MGLFeature> * (const mbgl::Feature &feature) {
-        GeometryEvaluator<double> evaluator(feature);
-        return mapbox::geometry::geometry<double>::visit(feature.geometry, evaluator);
+        NSMutableDictionary *attributes = [NSMutableDictionary dictionaryWithCapacity:feature.properties.size()];
+        for (auto &pair : feature.properties) {
+            auto &value = pair.second;
+            PropertyValueEvaluator evaluator;
+            attributes[@(pair.first.c_str())] = mbgl::Value::visit(value, evaluator);
+        }
+        
+        GeometryEvaluator<double> evaluator;
+        MGLShape <MGLFeaturePrivate> *shape = mapbox::geometry::geometry<double>::visit(feature.geometry, evaluator);
+        if (feature.id) {
+            shape.identifier = @(*feature.id);
+        }
+        shape.attributes = attributes;
+        return shape;
     });
     return [NSArray arrayWithObjects:&shapes[0] count:shapes.size()];
 }
-
-@implementation MGLPointFeature
-
-- (id)attributeForKey:(NSString *)key {
-    return self.attributes[key];
-}
-
-@end
-
-@implementation MGLPolylineFeature
-
-- (id)attributeForKey:(NSString *)key {
-    return self.attributes[key];
-}
-
-@end
-
-@implementation MGLPolygonFeature
-
-- (id)attributeForKey:(NSString *)key {
-    return self.attributes[key];
-}
-
-@end
-
-@implementation MGLMultiPointFeature
-
-- (id)attributeForKey:(NSString *)key {
-    return self.attributes[key];
-}
-
-@end
-
-@implementation MGLMultiPolylineFeature
-
-- (id)attributeForKey:(NSString *)key {
-    return self.attributes[key];
-}
-
-@end
-
-@implementation MGLMultiPolygonFeature
-
-- (id)attributeForKey:(NSString *)key {
-    return self.attributes[key];
-}
-
-@end
-
-@implementation MGLShapeCollectionFeature
-
-- (id)attributeForKey:(NSString *)key {
-    return self.attributes[key];
-}
-
-@end
diff --git a/platform/darwin/src/MGLFeature_Private.h b/platform/darwin/src/MGLFeature_Private.h
index 837461871e..fbc7f88559 100644
--- a/platform/darwin/src/MGLFeature_Private.h
+++ b/platform/darwin/src/MGLFeature_Private.h
@@ -4,4 +4,8 @@
 #import <mbgl/util/geo.hpp>
 #import <mbgl/util/feature.hpp>
 
+/**
+ Returns an array of `MGLFeature` objects converted from the given vector of
+ vector tile features.
+ */
 NS_ARRAY_OF(MGLShape <MGLFeature> *) *MGLFeaturesFromMBGLFeatures(const std::vector<mbgl::Feature> &features);