[
https://issues.apache.org/jira/browse/JEXL-60?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Henri Biestro updated JEXL-60:
------------------------------
Description:
While working on JEXL-35 and looking at Cobertua reports, it appeared that some
classes in jexl.util were not really used.
It seems that the intention in the original code line was to create specialized
executors for each type of property getters and setters but was never
completed. Thus, the Uberspectimpl was partly in charge of discovering methods
which made things harder to follow and maintain.
The patch completes the original o.a.c.jexl.util with specialized executors for
{property, map, list, duck} setters and getters as well as methods. Since 2.0
already switched the method cache from using string as keys to using
MethodKey(s), their usage has been promoted futher up (a method key is a method
name and a set of parameters - not arguments!-) so MethodExecutor(s) benefit
from their existence.
Since it seemed natural, IntrospectionUtils code has been displaced to
MethodKey since this is where parameters type matching has to occur.
The general method (setter & getter) discovery mechanism is reusing the
original algorithm, trying the different types and returning the first alive
one (aka the first executor for which a java.lang.method could be found).
The patch is faithfull to the original packages however some code has migrated;
most of the Uberspectimpl code has been displaced into
o.a.c.jexl.util.Introspector to keep it as simple as possible and still
derivable.
There were also some low-hanging fruits in this whole refactoring:
The AST nodes (now JexlNode & SimpleNode) carry a volatile value field that
allows to use them as cache for executors; this improves performance and allows
no locking concurrency. Since we are creating executors, we might as well reuse
when possible.
The Info (debugging info) has been put to use so Jexl can create more
meaningfull error message when they occur; putting the JexlEngine in debug mode
will make warning/exception messages report their point of creation (file or
url for scripts, method for expression).
And a 'new' function/operator has been implemented so Jexl can now instantiate
objects as well.
It is a huge and worrysome patch but the Checkstyle & Corbertua reports look
pretty good.
The following classes need to be removed *before* applying the patch:
src/java/org/apache/commons/jexl/parser/JEXLNode.java
src/java/org/apache/commons/jexl/Arithmetic.java
src/java/org/apache/commons/jexl/util/PropertyExecutor.java
src/java/org/apache/commons/jexl/util/BooleanPropertyExecutor.java
src/java/org/apache/commons/jexl/util/introspection/IntrospectionUtils.java
src/java/org/apache/commons/jexl/util/introspection/UberspectLoggable.java
src/java/org/apache/commons/jexl/util/GetExecutor.java
The pom.xml has been modified to use a newer version of the javacc plugin.
was:
While working on JEXL-35 and looking at Cobertua reports, it appeared that some
classes in jexl.util were not really used.
It seems that the intention in the original code line was to create specialized
executors for each type of property getters and setters but was never
completed. Thus, the Uberspectimpl was partly in charge of discovering methods
which made things harder to follow and maintain.
The patch completes the original o.a.c.jexl.util with specialized executors for
{property, map, list, duck} setters and getters as well as methods. Since 2.0
already switched the method cache from using string as keys to using
MethodKey(s), their usage has been promoted futher up (a method key is a method
name and a set of parameters - not arguments!-) so MethodExecutor(s) benefit
from their existence.
The general method (setter & getter) disovery mechanism is reusing the original
algorithm, trying the different types and returning the first alive one (aka
the first executor for which a java.lang.method could be found).
The patch is faithfull to the original packages however some code has migrated;
most of the Uberspectimpl code has been displaced into
o.a.c.jexl.util.Introspector to keep it as simple as possible and still
derivable.
There were also some low-hanging fruits in this whole refactoring:
The AST nodes (now JexlNode & SimpleNode) carry a volatile value field that
allows to use them as cache for executors; this improves performance and allows
no locking concurrency. Since we are creating executors, we might as well reuse
when possible.
The Info (debugging info) has been put to use so Jexl can create more
meaningfull error message when they occur; putting the JexlEngine in debug mode
will make warning/exception messages report their point of creation (file or
url for scripts, method for expression).
It is a huge and worrysome patch but the Checkstyle & Corbertua reports look
pretty good.
The following classes need to be removed *before* applying the patch:
src/java/org/apache/commons/jexl/parser/JEXLNode.java
src/java/org/apache/commons/jexl/Arithmetic.java
src/java/org/apache/commons/jexl/util/PropertyExecutor.java
src/java/org/apache/commons/jexl/util/BooleanPropertyExecutor.java
src/java/org/apache/commons/jexl/util/introspection/UberspectLoggable.java
src/java/org/apache/commons/jexl/util/GetExecutor.java
The pom.xml has been modified to use a newer version of the javacc plugin.
> Refactor o.a.c.jexl.util and o.a.c.jexl.util.introspection
> ----------------------------------------------------------
>
> Key: JEXL-60
> URL: https://issues.apache.org/jira/browse/JEXL-60
> Project: Commons JEXL
> Issue Type: Improvement
> Affects Versions: 2.0
> Reporter: Henri Biestro
> Priority: Blocker
> Fix For: 2.0
>
> Attachments: JEXL-60.patch, jexl60.tar.gz
>
>
> While working on JEXL-35 and looking at Cobertua reports, it appeared that
> some classes in jexl.util were not really used.
> It seems that the intention in the original code line was to create
> specialized executors for each type of property getters and setters but was
> never completed. Thus, the Uberspectimpl was partly in charge of discovering
> methods which made things harder to follow and maintain.
> The patch completes the original o.a.c.jexl.util with specialized executors
> for {property, map, list, duck} setters and getters as well as methods. Since
> 2.0 already switched the method cache from using string as keys to using
> MethodKey(s), their usage has been promoted futher up (a method key is a
> method name and a set of parameters - not arguments!-) so MethodExecutor(s)
> benefit from their existence.
> Since it seemed natural, IntrospectionUtils code has been displaced to
> MethodKey since this is where parameters type matching has to occur.
> The general method (setter & getter) discovery mechanism is reusing the
> original algorithm, trying the different types and returning the first alive
> one (aka the first executor for which a java.lang.method could be found).
> The patch is faithfull to the original packages however some code has
> migrated; most of the Uberspectimpl code has been displaced into
> o.a.c.jexl.util.Introspector to keep it as simple as possible and still
> derivable.
> There were also some low-hanging fruits in this whole refactoring:
> The AST nodes (now JexlNode & SimpleNode) carry a volatile value field that
> allows to use them as cache for executors; this improves performance and
> allows no locking concurrency. Since we are creating executors, we might as
> well reuse when possible.
> The Info (debugging info) has been put to use so Jexl can create more
> meaningfull error message when they occur; putting the JexlEngine in debug
> mode will make warning/exception messages report their point of creation
> (file or url for scripts, method for expression).
> And a 'new' function/operator has been implemented so Jexl can now
> instantiate objects as well.
> It is a huge and worrysome patch but the Checkstyle & Corbertua reports look
> pretty good.
> The following classes need to be removed *before* applying the patch:
> src/java/org/apache/commons/jexl/parser/JEXLNode.java
> src/java/org/apache/commons/jexl/Arithmetic.java
> src/java/org/apache/commons/jexl/util/PropertyExecutor.java
> src/java/org/apache/commons/jexl/util/BooleanPropertyExecutor.java
> src/java/org/apache/commons/jexl/util/introspection/IntrospectionUtils.java
> src/java/org/apache/commons/jexl/util/introspection/UberspectLoggable.java
> src/java/org/apache/commons/jexl/util/GetExecutor.java
> The pom.xml has been modified to use a newer version of the javacc plugin.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
