On Sat, Sep 14, 2013 at 8:16 AM, sebb <[email protected]> wrote: > 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? >
Because one uses the value of the other. Gary > > > 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] > > -- 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
