Getting rid of the Script helper abstractions

[Mathieu Lirzin]

Hello,

While trying to understand the Groovy engine implemenation details, I
discovered the mechanism used to add some bindings to scripts. Those
bindings are meant to make it convenient for programmers to write those
scripts in the context of OFBiz by letting them invoke a service and
query the database easily.

This feature has been introduced in 2012 by Adrian Crum while he was
working on the Script Engine which allows OFBiz to interact generically
with scripting languages implementing the JSR-223 specification. [1]

Adrian chose to use the Abstract Factory pattern combined with the
service loader [2] for an obscure reason since there is only one
implementation of the ‘ScriptHelperFactory’ and only one implementation
of the ‘ScriptHelper’ interface.  This give me the feeling that this set
of abstraction is unnecessary and only make things harder to reason
about.

As a consequence I propose to simplify the implementation by removing
the abstract factory pattern and merging the ‘ScriptHelper’ interface
with its sole implementation.

What do people think?

[1] https://github.com/apache/ofbiz/commit/39eaa0b48d836b143a2c03f44654cd1cab7f031f
[2] https://docs.oracle.com/javase/8/docs/api/java/util/ServiceLoader.html

--
Mathieu Lirzin
GPG: F2A3 8D7E EB2B 6640 5761  070D 0ADE E100 9460 4D37

[taher]

Design patterns gone awry :) Anyway, I don't see the need to delete
the interface, just instantiate it without a factory.

On Sun, Jan 13, 2019 at 11:20 PM Mathieu Lirzin
<[hidden email]> wrote:

>
> Hello,
>
> While trying to understand the Groovy engine implemenation details, I
> discovered the mechanism used to add some bindings to scripts. Those
> bindings are meant to make it convenient for programmers to write those
> scripts in the context of OFBiz by letting them invoke a service and
> query the database easily.
>
> This feature has been introduced in 2012 by Adrian Crum while he was
> working on the Script Engine which allows OFBiz to interact generically
> with scripting languages implementing the JSR-223 specification. [1]
>
> Adrian chose to use the Abstract Factory pattern combined with the
> service loader [2] for an obscure reason since there is only one
> implementation of the ‘ScriptHelperFactory’ and only one implementation
> of the ‘ScriptHelper’ interface.  This give me the feeling that this set
> of abstraction is unnecessary and only make things harder to reason
> about.
>
> As a consequence I propose to simplify the implementation by removing
> the abstract factory pattern and merging the ‘ScriptHelper’ interface
> with its sole implementation.
>
> What do people think?
>
> [1] https://github.com/apache/ofbiz/commit/39eaa0b48d836b143a2c03f44654cd1cab7f031f
> [2] https://docs.oracle.com/javase/8/docs/api/java/util/ServiceLoader.html
>
> --
> Mathieu Lirzin
> GPG: F2A3 8D7E EB2B 6640 5761  070D 0ADE E100 9460 4D37