Hi Sergiu, Thanks for fixing my javadoc :) However our automated refactoring has some shortcomings which I've pointed below. I don't know if we can improve the IDE configuration but it would be nice to try since it generates some code that is less readable than the one we generate by hand. [snip] On Jun 4, 2008, at 3:27 AM, sdumitriu (SVN) wrote: [snip]

-public class DefaultVelocityContextFactory extends AbstractLogEnabled - implements VelocityContextFactory, Initializable, Composable +public class DefaultVelocityContextFactory extends AbstractLogEnabled implements + VelocityContextFactory, Initializable, Composable

IMO it's better to keep implements on the same line as the interfaces it implements and not break it.

// Instantiate Velocity tools if (this.properties != null) { - for (Enumeration< ? > props = this.properties.propertyNames(); - props.hasMoreElements();) - { + for (Enumeration< ? > props = this.properties.propertyNames(); props + .hasMoreElements();) {

This is way less readable than the version I had. Also remember that we agreed to use a curly brace on the next line for this case (i.e. when the conditions wraps). In addition I don't like how props.hasMoreElements() is cut.

// Call all components implementing the VelocityContextInitializer's role. try { - for (Object interceptor - : this.componentManager.lookupList(VelocityContextInitializer.ROLE)) - { + for (Object interceptor : this.componentManager + .lookupList(VelocityContextInitializer.ROLE)) { ((VelocityContextInitializer) interceptor).initialize(context); }

same here for the curly brace.

// Ensure that initialization has been called if (this.engine == null) { - throw new XWikiVelocityException("This Velocity Engine has not yet been initialized. " - + " You must call its initialize() method before you can use it."); + throw new XWikiVelocityException( + "This Velocity Engine has not yet been initialized. " + + " You must call its initialize() method before you can use it.");

I think my version is better and takes only 2 lines.