search

schxslt / schxslt-java

Java classes for Schematron validation with SchXslt
MIT License
7 stars 4 forks source link

"with" methods return same instance, following builder pattern #43

Closed eduarddrenth closed 4 years ago

dmj commented 4 years ago

Thanks for the contribution! I deliberately chose not to implement setters for properties but functional constructors (with*) to make an instance immutable.

eduarddrenth commented 4 years ago

Thanks for the contribution! I deliberately chose not to implement setters for properties but functional constructors (with*) to make an instance immutable.

I like your with... design, but...

In Java-land you would expect a builder pattern, so users of your lib would do:

new SchematroN(..).with...(...).with(...)

In your solution this would work perfectly, but create three Schematron objects instead of one.

Users of your lib can determine for themselves to reuse a Schematron object or not. It is more important for Schematron to be threadsafe than to be immutable. And if you want it to be immutable, you'll have to do more than let you with* methods return a new Schematron, for example private final fields, the use of Collections.unmodifiable and no exposure of arrays (elements can be changed)

dmj commented 4 years ago

You are right: Just providing with* constructors does not make the object immutable. I also agree that providing a thread safe implementation is an important goal. I created an issue #44 and a dedicated branch https://github.com/schxslt/schxslt-java/tree/t/thread-safety to work on this.

I will keep the constructors, though. They contribute to the goal of thread safety and returning a the same instance would be an incompatible change to public API. Changing the public API in this way would require a 3.x release.

eduarddrenth commented 4 years ago

understandable, good to hear about #44