directory-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Emmanuel Lecharny <elecha...@gmail.com>
Subject Re: svn commit: r596822 - /directory/shared/branches/bigbang/ldap/src/main/java/org/apache/directory/shared/ldap/name/LdapDnParser.java
Date Tue, 20 Nov 2007 21:38:29 GMT
Felix Knecht wrote:
> elecharny@apache.org schrieb:
>   
>> Author: elecharny
>> Date: Tue Nov 20 12:53:15 2007
>> New Revision: 596822
>>
>> URL: http://svn.apache.org/viewvc?rev=596822&view=rev
>> Log:
>> Slightly improved Felix fix : Instead of dumping bytes as if they were just bytes
for a DN, translating this byte array back to a String.
>> (the DN is always a valid byte array representation of a String )
>>     
>
> I'm still having a huge lack of knowledge. Thanks for reviewing my commits :)
>   
Plain normal ! FYI, whenmanipulation strings and byte[], we have some 
helper methods in StringTools. They should be used to convert String to 
byte[] or byte[] to String to avoid a try {...} catch (...). I just 
jumped on your fix and 'improved it', and trust me, if you didn't fix it 
at first, it would have staid as is for a while ;)

StringTools.Utf8ToString( byte[] )
and
StringTools.getBytesUtf8( String )

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 )
- 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 !).

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

Thanks !
> Felix
>
>   


-- 
--
cordialement, regards,
Emmanuel L├ęcharny
www.iktek.com
directory.apache.org



Mime
View raw message