Remove GeometryServiceProvider.Instance about geoapi HOT 4 CLOSED

Just to add a few more comments, continuing the discussion with @bricelam cited above...

I think it may make sense to retain GeometryServiceProvider.Instance. As written above, the Npgsql ADO provider (not the EF Core provider) has no dependency injection of any sort. Now, I think the following are reasonable requirements from the Npgsql NTS plugin:

1. If the user hasn't indicated they're interested in any specific GeoAPI implementation, the plugin should default to using NetTopologySuite (i.e. instantiate NTS objects when reading PostGIS data from the database).
2. However, the plugin should definitely support other GeoAPI implementations. This could be done by:
   a. Receiving a GeometryServiceProvider instance in the plugin's initialization, or
   b. A global static setting as it is now (GeometryServiceProvider.Instance)

While I agree that responsibilities around setting GeometryServiceProvider.Instance can be a bit confusing, it does seem to make sense to allow end users to specify the desired GeoAPI implementation globally once, at the start of the application, and have it picked up by various libraries (e.g. ORMs). However, these libraries should probably also optionally allow a GeometryServiceProvider instance to be passed in, just in case the global setting isn't sufficient.

I may have missed some issues/complexities that GeometryServiceProvider.Instance creates, though...

from geoapi.

Copying some comments here.

@bricelam said:

We never go through GeoAPI.GeometryServiceProvider.Instance so [calling NetTopologySuiteBootstrapper.Bootstrap() is] not needed. Instead, we register NtsGeometryServices.Instance (as IGeometryServices) in the service provider and pass it into the NTS Reader.

This provides much more flexibility than the GeometryServiceProvider.Instance service locator anti-pattern since you can use different services with different DbContext instances.

@roji said:

I think this is where the SqlServer/Sqlite spatial implementation may actually have an advantage over Npgsql's.

With Npgsql, NTS is supported at the ADO level - which is great because you can easily use it without EF Core. However, unlike with EF Core there's no fancy dependency injection in ADO, so at least at the moment everything is done with the global NTS bootstrapping, which as you say isn't ideal.

from geoapi.

I think it may make sense to retain GeometryServiceProvider.Instance. As written above, the Npgsql ADO provider (not the EF Core provider) has no dependency injection of any sort. Now, I think the following are reasonable requirements from the Npgsql NTS plugin:

1. If the user hasn't indicated they're interested in any specific GeoAPI implementation, the plugin should default to using NetTopologySuite (i.e. instantiate NTS objects when reading PostGIS data from the database).
2. However, the plugin should definitely support other GeoAPI implementations. This could be done by:
   a. Receiving a GeometryServiceProvider instance in the plugin's initialization, or
   b. A global static setting as it is now (GeometryServiceProvider.Instance)

The idea behind this one is that I expect everybody who currently uses GeometryServiceProvider.Instance to fall into at least one of two categories (probably often both):

1. They would be better off using an implementation of IoC that's already available in their context to get the IGeometryServices instead (this is what @bricelam was referring to), and/or
2. They can just as easily switch to using NtsGeometryServices.Instance instead.

With #70, the case is even easier to make because the two types are going to be right next to each other.

from geoapi.

With GeoAPI merged into NetTopologySuite there is no reason to have two public instances of IGeometryServices.

from geoapi.