On 14 September 2013 13:09, Gary Gregory <[email protected]> wrote: > On Sat, Sep 14, 2013 at 5:44 AM, sebb <[email protected]> wrote: > >> On 14 September 2013 03:13, <[email protected]> wrote: >> > Author: ggregory >> > Date: Sat Sep 14 02:13:01 2013 >> > New Revision: 1523175 >> > >> > URL: http://svn.apache.org/r1523175 >> > Log: >> > No need to lazy-init statics contextFactory and compilationContext, make >> them final. >> >> Is the class always needed by applications? >> > > Yes, you always need a JXPathContext.contextFactory, from > https://commons.apache.org/proper/commons-jxpath/*:* > > Address address = > (Address)JXPathContext.newContext(vendor).getValue("locations[address/zipCode='90210']/address"); > > The above could avoid casting with generics (assuming the call site > knows what it is doing of course). > > For JXPathContext.compilationContext, it depends on whether the app > calls JXPathContext.compile(String), but I prefer consistency of the > init pattern here... It's debatable, sure.
If compilationContext is not always needed, then the patch changes the behaviour. Using IODH would avoid changing the behaviour, but is slightly more involved. What about the question of the order of the init? Why is that important? > Gary > > > > > >> >> If not, then using IODH would be better. >> >> > Modified: >> > >> commons/proper/jxpath/trunk/src/java/org/apache/commons/jxpath/JXPathContext.java >> > >> > Modified: >> commons/proper/jxpath/trunk/src/java/org/apache/commons/jxpath/JXPathContext.java >> > URL: >> http://svn.apache.org/viewvc/commons/proper/jxpath/trunk/src/java/org/apache/commons/jxpath/JXPathContext.java?rev=1523175&r1=1523174&r2=1523175&view=diff >> > >> ============================================================================== >> > --- >> commons/proper/jxpath/trunk/src/java/org/apache/commons/jxpath/JXPathContext.java >> (original) >> > +++ >> commons/proper/jxpath/trunk/src/java/org/apache/commons/jxpath/JXPathContext.java >> Sat Sep 14 02:13:01 2013 >> > @@ -380,8 +380,17 @@ import org.apache.commons.jxpath.util.Ke >> > * @version $Revision$ $Date$ >> > */ >> > public abstract class JXPathContext { >> > - private static volatile JXPathContextFactory contextFactory; >> > - private static volatile JXPathContext compilationContext; >> > + >> > + static { >> > + // Initialize in this order: >> >> Why is it necessary to use this order? >> The reason should be documented. >> >> > + // 1) contextFactory >> > + contextFactory = JXPathContextFactory.newInstance(); >> > + // 2) compilationContext >> > + compilationContext = JXPathContext.newContext(null); >> > + } >> > + >> > + private static final JXPathContextFactory contextFactory; >> > + private static final JXPathContext compilationContext; >> >> I'm suprised that the compiler does not complain that the fields are >> not defined before they are used in the static{} block. >> >> I'd prefer to see the definitions before the static block. >> >> > private static final PackageFunctions GENERIC_FUNCTIONS = >> > new PackageFunctions("", null); >> > @@ -435,9 +444,6 @@ public abstract class JXPathContext { >> > * @return JXPathContextFactory >> > */ >> > private static JXPathContextFactory getContextFactory () { >> > - if (contextFactory == null) { >> > - contextFactory = JXPathContextFactory.newInstance(); >> > - } >> > return contextFactory; >> > } >> > >> > @@ -646,9 +652,6 @@ public abstract class JXPathContext { >> > * @return CompiledExpression >> > */ >> > public static CompiledExpression compile(String xpath) { >> > - if (compilationContext == null) { >> > - compilationContext = JXPathContext.newContext(null); >> > - } >> > return compilationContext.compilePath(xpath); >> > } >> > >> > >> > >> >> --------------------------------------------------------------------- >> To unsubscribe, e-mail: [email protected] >> For additional commands, e-mail: [email protected] >> >> > > > -- > E-Mail: [email protected] | [email protected] > Java Persistence with Hibernate, Second > Edition<http://www.manning.com/bauer3/> > JUnit in Action, Second Edition <http://www.manning.com/tahchiev/> > Spring Batch in Action <http://www.manning.com/templier/> > Blog: http://garygregory.wordpress.com > Home: http://garygregory.com/ > Tweet! http://twitter.com/GaryGregory --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
