Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Converter attribute ("StjGeometryConverter") #61

Closed
plettb opened this issue Sep 30, 2020 · 2 comments
Closed

Converter attribute ("StjGeometryConverter") #61

plettb opened this issue Sep 30, 2020 · 2 comments
Labels
STJ Applies to NetTopologySuite.IO.GeoJSON4STJ

Comments

@plettb
Copy link

plettb commented Sep 30, 2020

I would really like to be able to decorate a property in my class with the converter. For example:
[JsonConverter(typeof(GeometryConverter))]
public Geometry Location { get; set; }
The converter I'd like to use is "StjGeometryConverter", but it is only available internally or via a factory ("GeoJsonConverterFactory").

Is there a way to use that converter in an attribute, even if it is only exposed by a factory?

Alternatively, is there a reason that converter isn't public?

@airbreather airbreather reopened this Oct 1, 2020
@airbreather airbreather added the STJ Applies to NetTopologySuite.IO.GeoJSON4STJ label Oct 1, 2020
@airbreather
Copy link
Member

image

Sorry about that ^, misclicked

Related: #63, #56

Is there a way to use that converter in an attribute, even if it is only exposed by a factory?

The recommended approach is to add an instance of the factory to JsonSerializerOptions.Converters, since that makes it more straightforward to change the constructor call if you eventually find that you need / want to, since you would only theoretically need to touch one place.

However, you should actually be able to use [JsonConverter(typeof(GeoJsonConverterFactory))], since it inherits from JsonConverterFactory, which itself inherits from JsonConverter. I haven't tried this out.

Alternatively, is there a reason that converter isn't public?

For a library like this, in general, I am a proponent of keeping public APIs to a minimum. This means less that we need to document and (in theory) it makes it easier to change implementation details.

In this specific case, it does that, and it also should make it easier to fall into the "pit of success": if we expose just the one converter (in this case, GeoJsonConverterFactory), then you will have access to not only the Geometry parts of the GeoJSON spec, but also the Feature and FeatureCollection parts of it, which let you associate data with each Geometry.

@plettb
Copy link
Author

plettb commented Oct 1, 2020

[JsonConverter(typeof(GeoJsonConverterFactory))]

worked like a charm!!!

I agree on keeping as much internal as possible, and since this works, I even agree with keeping that converter internal. :)

Thanks a lot!

@plettb plettb closed this as completed Oct 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
STJ Applies to NetTopologySuite.IO.GeoJSON4STJ
Projects
None yet
Development

No branches or pull requests

2 participants