mapbox/turf-swift

Feature should conform to Identifiable

1ec5 opened this issue · 1 comments

1ec5 commented

The Swift standard library defines an Identifiable protocol that provides for an id property. The documentation for this property says the ID’s value can be as unique or as commonplace as the containing type defines it to be. That would make it well-suited for the id property on Feature objects in GeoJSON.

Turf currently stores this property in Feature.identifier. Perhaps we should rename identifier to id and have Feature conform to Identifiable. This would make the identifier more discoverable. However, GeoJSON feature IDs are optional, so the ID associated type would need to be Optional<FeatureIdentifier>, which is probably not what most Identifiable consumers would expect.

The name identifier follows the Cocoa naming convention that abbreviations such as “ID” should always be spelled out. To my knowledge, the Cocoa convention was motivated by the need to avoid collisions with the Objective-C keyword id and the awkwardness of a property named ID, capitalized according to another Cocoa convention. These considerations are less relevant to Turf, which has never supported Objective-C. The Swift naming guidelines make an exception for “embracing precedent”, and there’s plenty of precedent for “ID”.

To maintain backwards compatibility, we could retain the identifier property as a deprecated computed property based on id. It’s unclear if the Identifiable conformance would be a backwards-incompatible change, but we could probably get away with it because Identifiable is still seldom used at this point.

/cc @mapbox/navigation-ios @mapbox/maps-ios

1ec5 commented

Per @bamx23, Identifiable is used by SwiftUI.