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.

_______________________________________________
devs mailing list
[email protected]
http://lists.xwiki.org/mailman/listinfo/devs

Reply via email to