Generic GEOSParameter type in CAPI

[pramsey]

The CAPI already has two parameter types GEOSBufferParams and GEOSMakeValidParams along with a small herd of setters and getters for those types. As more GEOS functions with fun collections of optional parameters show up (the hull functions had more than we exposed) it feels like it would be cleaner to have a single GEOSParams to handle all those cases.

GEOSParams_create();
GEOSParams_getDouble(const char* paramName, &double value);
GEOSParams_getInteger(const char* paramName, &int value);
GEOSParams_setDouble(const char* paramName, double value);
GEOSParams_setInteger(const char* paramName, int value);
GEOSParams_destroy();

[strk]

Sounds like a great idea to me

[dr-jts]

I'm lukewarm on this idea, since I prefer strong typing and compiler enforcement. The generic approach punts the complexity to documentation and the function implementor.

If this approach is followed, would invalid parameters be detected in some way? Or. just ignore them and use defaults?

Are there any patterns which other APIs use to handle this?

This is an issue in JTS as well, so there is pressure to keep things simple in that API as well.

For the specific case of GEOSPolygonHullSimplify, there is an alternate parameter "areaDeltaRatio" (instead of "vertexNumFraction"). To expose this I suggest adding an API function GEOSPolygonHullSimplifyByArea, with the alternate parameter interpretation.

Although this expands the API slightly, and the name is a bit cumbersome, I'm not sure this is enough to drive using a generic parameters API. Maybe there will be something in the future which is complicated enough to warrant it.

On Mon, Jun 6, 2022 at 2:38 PM Paul Ramsey @.***> wrote:

The CAPI already has two parameter types GEOSBufferParams and GEOSMakeValidParams along with a small herd of setters and getters for those types. As more GEOS functions with fun collections of optional parameters show up (the hull functions had more than we exposed) it feels like it would be cleaner to have a single GEOSParams to handle all those cases.

GEOSParams_create(); GEOSParams_getDouble(const char paramName, &double value); GEOSParams_getInteger(const char paramName, &int value); GEOSParams_setDouble(const char paramName, double value); GEOSParams_setInteger(const char paramName, int value); GEOSParams_destroy();

— Reply to this email directly, view it on GitHub https://github.com/libgeos/geos/issues/627, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA25SXKWVSIFNSOBYN5N2ZTVNZVUPANCNFSM5YAZBROQ . You are receiving this because you are subscribed to this thread.Message ID: @.***>

[pramsey]

As applied to the new polygon hull function, for example, see https://github.com/pramsey/geos/blob/main-params/capi/geos_c.h.in#L3669-L3671

[dr-jts]

Another concern I have is verbose code for the client. Instead of a one-line call the caller will have to:

• create a param object
• populate it
• call the operation with it
• free the param object

See GEOSBufferParams test for an example (which doesn't include the free).

[dbaston]

Instead of a one-line call

I don't think it has to be an either/or - you can cover common cases with functions like GEOSBuffer and use a params object for more esoteric configuration.

[dr-jts]

I don't think it has to be an either/or - you can cover common cases with functions like GEOSBuffer and use a params object for more esoteric configuration.

For sure. But that raises the question of when should a generic parameter block be introduced? The reason it is used in buffer is that there are effectively several different signatures depending on which which combination of styles is desired. Also, there is a definite possibility that more style parameters will be introduced (in fact, I have a nascent PR to add some!)

For GEOSPolygonHullSimplifier (to use a current case) the situation is simpler. All uses can be covered by 3 parameters:

• isOuter boolean to indicate hull type
• a code to indicate which kind of control parameter is being provided (currently vertex count fraction or area delta ratio - but conceivably some more will be added, such as absolute target vertex count or area delta)
• the control parameter value

So this can be handled by a single signature. (@pramsey: perhaps we should change to having a single signature, and add an integer controlParamType code, with constants provided for the supported values?)

[strk]

On Fri, Jun 10, 2022 at 09:42:26AM -0700, Martin Davis wrote:

Another concern I have is verbose code for the client. Instead of a one-line call the caller will have to:

That's the price we pay for a stable and extensible API. I would not get rid of it.

--strk;

[pramsey]

I don't think we managed to convince ourselves this juice was worth the squeeze.