directory-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Felix Knecht <fel...@apache.org>
Subject Re: svn commit: r596822 - /directory/shared/branches/bigbang/ldap/src/main/java/org/apache/directory/shared/ldap/name/LdapDnParser.java
Date Wed, 21 Nov 2007 05:38:54 GMT

> 
> Regarding some of your other commits, I have a few comments :
> 
> - be carefull to use the same style everywhere. I can see some Sun style
> when we are not following this style (we don't start a { at the end of a
> line )

Thanks for spotting. I saw it but seem to have missed it on some place. C&P is nice but
also dangerous ...

> - you have changed a lot of setters and getters by using a copy for
> mutable fields. This is good but sometime, you also have to be careful
> with this approach : for instance, in the Codec package, the objects are
> created temporarily, before being transfered to another object.
> Introducing a copy here cost and have no advantage, because the data are
> never modified.
> 
> I would suggest that you simply add some @Todo if you see such 'bad
> habbits' : they may have been introduced on purpose (certainly not the
> case for all of them : you are catching a lot of potential errors !).

There can be (a lot) more says my tool.

I'm not sure if it is really a good idea to add @Todo all over the place and leave the work
up to you to find all the
Todos I added including the more or less meaningful descriptions of what could be done.

The tool I use is an eclipse plugin called findbugs (http://findbugs.sourceforge.net/) and
it also helped me in other
projects to find those potential risk and other always obvious things, i.e.
Integer foo = new Integer(1);
is slower than
Integer foo = Integer.valueOf(1);

> 
> Don't be scared anyway, we are double checking, just in case.

Hehe - that's one of the risks a community has when voting as new committer ;-)

Felix

> 
> Thanks !
>> Felix
>>
>>   
> 
> 


Mime
View raw message