Changes in API-interfaces regarding return types

[sszuev]

I think, it would be better to replace all the concrete modifiable classes from interfaces with interfaces as well. Basically, it's about the return types mentioned in the org.semanticweb.owlapi.model.OWLOntologyManager, i.e. about the followings:

• org.semanticweb.owlapi.model.OntologyConfigurator (ONT-API has extended configuration)
• org.semanticweb.owlapi.model.OWLOntologyLoaderConfiguration
• org.semanticweb.owlapi.model.OWLOntologyWriterConfiguration
• org.semanticweb.owlapi.util.PriorityCollection (or replace it with java.util.Collection)

I think this also would be more correct in terms of architecture\design: it is better when interface methods do not return mutable non-final concrete classes.

In order not to interfere with the current client code, this could be done somewhere in version 5.2+ v6.x.x, I think, implies more deep changes, while these proposed changes may not be even visible to end-users (to notice, it is need to have code calling constructors directly). And also, generally speaking, it would be better to make a generic mechanism to retrieve\set configuration that would not depend on implementation at all.

---

Originally it was posted here: https://github.com/orgs/owlcs/teams/ontapi-contributors/discussions/1

[ignazio1977]

a generic mechanism to retrieve\set configuration that would not depend on implementation at all

I like that idea. What would this look like?

[sszuev]

I was thinking of something like this:

    interface OWLOntologyManager {
        OntologyConfig getOntologyConfigurator();
    }
    interface OntologyConfig {

        OntologyConfig setBoolean(Key key, boolean value);

        boolean getBoolean(Key key);

        @Deprecated // ?
        default OntologyConfig setAcceptingHTTPCompression(boolean b) {
            return setBoolean(OWLAPISettings.BOOLEAN_ACCEPT_HTTP_COMPRESSION, b);
        }

        @Deprecated // ?
        default boolean shouldAcceptHTTPCompression() {
            return getBoolean(OWLAPISettings.BOOLEAN_ACCEPT_HTTP_COMPRESSION);
        }

        interface Key {
        }
    }
    enum OWLAPISettings implements OntologyConfig.Key {
        BOOLEAN_ACCEPT_HTTP_COMPRESSION,
    }

I think, such a configuration should be much easier to override and extend across implementations. (to be more specific, I should probably prepare a PR ?)

[ignazio1977]

Why not simplify further and just have a Map<String, Object>? ConfigurationOptions can act as the list of known properties and be easily adapted to expose the property name (it already has that information to look up properties from property files and system properties), and any implementations specific property can be added without impact. ConfigurationOptions would just be there for defaults and to avoid mistyping property names.

[sszuev]

Why not simplify further and just have a Map<String, Object>? ConfigurationOptions can act as the list of known properties and be easily adapted to expose the property name (it already has that information to look up properties from property files and system properties), and any implementations specific property can be added without impact. ConfigurationOptions would just be there for defaults and to avoid mistyping property names.

I think that wouldn't be very user-friendly:

• I think, java.util.Map is inconvenient structure here and even dangerous: we should not encourage clients to use raw-types: in v5 config setting values can be Boolean, String, Enum, and even arbitrary objects (e.g. in ontapi).

• There are now three different configuration types: one mutable manager-wide config and a pair of load \& store immutable configs for every ontology. So the current behavior is quite complicated. I'm not sure it is possible to re-implement it using only JDK Maps (or even Properties)

• Also, any object in OWLAPI implements Serializable, that also may lead to some difficulties for end-users.

• In contrast to this, the pattern above is safe and does not require any change in the client code (unless they explicitly use constructors to initialize that configs - which, I think, should be rare). Also, it can be applied right now with comparably little efforts.

But on the other hand, I agree that the configuration management code is overly complicated, and switching to use java.util.Map would partially solve this problem.