Re: [xwiki-devs] [xwiki-notifications] r10096 - xwiki-platform/core/trunk/xwiki-velocity/src/main/java/org/xwiki/velocity - xwiki-devs@xwiki.org

4 Jun 2008

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.