search

synw / geojson

Utilities to work with geojson data in Dart
MIT License
36 stars 53 forks source link

missing GeojsonGeometry class #8

Closed ramsestom closed 5 years ago

ramsestom commented 5 years ago

all geometry classes (GeojsonPoint, GeojsonLine, GeojsonMultiLine, GeojsonPolygon, ...) should extend the same GeojsonGeometry abstract class to respect the geojson hierarchy and allow to point to a generic geojson geometry in our code if needed (for example if I want to define a class attribute as a geojson geometry without specifying the type because it is variable or unknown yet, I can't with the current lib implementation)

synw commented 5 years ago

generic geojson geometry in our code if needed (for example if I want to define a class attribute as a geojson geometry without specifying the type

Do you have a use case? I don't really understand what you want to achieve, a concrete example would help

ramsestom commented 5 years ago

Sure. Imagine you want to store contries with their boundaries as objects into your app. For some countries, the territory area is defined as a 'Polygon' whereas for others it is defined as a 'MultiPolygon' (for exmple France is composed of "metropolitan France" + "oversea territories" so it is a multipolygon whereas countries like Switzerland is just a unique polygon). Now, with the current lib implementation, you can't have an 'area' field into your 'Country' object (exept if you define it as dynamic) because the type of this field can be either a GeojsonPolygon or a GeojsonMultiPolygon. Hence the interest of having a generic GeojsonGeometry that all geojson geometry objects inherit (this 'geometry' hierarchy level is part of the geojson specs anyway so it should be in the lib)

synw commented 5 years ago

It makes sense and can be more flexible for developers. I ususally tend to use normalized data, like just multipolygons for countries so I did not bother with mixed geometry cases but yes more flexibility can't hurt. I'll have to double check the parser to ensure that it will work well in such cases: actually it is optimized for normalized data. It could be the occasion to support GeometryCollection that is missing actually

this 'geometry' hierarchy level is part of the geojson specs anyway so it should be in the lib

I'll take the time to read the spec more carefully before starting this refactor. Thanks for mentioning this. This package should stay close to the spec. Feel free to help with the code if you have ideas, motivation and time

synw commented 5 years ago

I tried to implement a GeoJsonGeometry class. It does not sound to be a good option. In the spec this object is basically just a geometry type and some coordinates. It introduces a level of abstraction in the code and some type conversions that are not worth it. The hypothetical benefits of this approach do not justify to introduce some complexity that will be a maintenance debt at some point