4 Jun
2008
4 Jun
'08
9:23 a.m.
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.