search

mapbox / geometry.hpp

C++ geometry types
ISC License
92 stars 37 forks source link

Moved feature.hpp to depend on geometry.hpp and not vise versa #52

Closed flippmoke closed 6 years ago

flippmoke commented 6 years ago

feature.hpp is currently embedded as part of geometry.hpp. This is not ideal because including geometry.hpp shouldn't require a feature object. It seems odd as well to have it under the mapbox::geometry namespace.

This change moves the feature.hpp to the mapbox::feature namespace and moves the file to be included at #include <mapbox/feature.hpp>

/cc @artemp

flippmoke commented 6 years ago

@artemp I am not aware of any code specifically using mapbox::geometry::feature that is actually widely used. Additionally, I feel with other changes that we might look into going forward (and likely will not be backwards compatible) we should not continue to support the previous namespace.

jfirebaugh commented 6 years ago

Please keep both feature and feature_collection in the geometry namespace. They're both used by Mapbox GL.

flippmoke commented 6 years ago

Merging this in for experimental use in the algorithms branch. Lots of different concepts here that will be tested before being considered for merge into master.

springmeyer commented 6 years ago

I'm also 👍 on experimental use of this change in the algorithms branch per @flippmoke's plan above.

I will however additionally offer that I took a look at what it would take to port Mapbox GL to this change. If we chose not to keep the namespace mapbox::geometry::feature (per @artemp suggestion) I wondered what would be needed to update. These were the changes needed:

The changes, while significant in lines of code, were easy to make. Overall my impression is that the change forces a nice separation of concerns.