You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@directory.apache.org by Felix Knecht <fe...@apache.org> on 2010/06/07 17:38:24 UTC
Re: svn commit: r952265 - /directory/apacheds/trunk/jdbm/src/main/java/jdbm/btree/BTree.java
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 06/07/10 17:20, elecharny@apache.org wrote:
> Author: elecharny
> Date: Mon Jun 7 15:20:46 2010
> New Revision: 952265
>
> URL: http://svn.apache.org/viewvc?rev=952265&view=rev
> Log:
> Reverted a wrong check on the keySerializer.
>
> Modified:
> directory/apacheds/trunk/jdbm/src/main/java/jdbm/btree/BTree.java
>
> Modified: directory/apacheds/trunk/jdbm/src/main/java/jdbm/btree/BTree.java
> URL: http://svn.apache.org/viewvc/directory/apacheds/trunk/jdbm/src/main/java/jdbm/btree/BTree.java?rev=952265&r1=952264&r2=952265&view=diff
> ==============================================================================
> --- directory/apacheds/trunk/jdbm/src/main/java/jdbm/btree/BTree.java (original)
> +++ directory/apacheds/trunk/jdbm/src/main/java/jdbm/btree/BTree.java Mon Jun 7 15:20:46 2010
> @@ -55,13 +55,13 @@ import java.io.Serializable;
> import java.util.Comparator;
> import java.util.concurrent.atomic.AtomicInteger;
>
> -import org.apache.directory.server.i18n.I18n;
> -
> import jdbm.RecordManager;
> import jdbm.helper.Serializer;
> import jdbm.helper.Tuple;
> import jdbm.helper.TupleBrowser;
>
> +import org.apache.directory.server.i18n.I18n;
> +
>
> /**
> * B+Tree persistent indexing data structure. B+Trees are optimized for
> @@ -206,12 +206,12 @@ public class BTree<K, V> implements Exte
> throw new IllegalArgumentException( I18n.err( I18n.ERR_519 ) );
> }
>
> - if ( keySerializer != null )
> + if ( ( keySerializer != null ) && ! ( keySerializer instanceof Serializable ) )
> {
> throw new IllegalArgumentException( I18n.err( I18n.ERR_520 ) );
> }
>
> - if ( valueSerializer != null && !( valueSerializer instanceof Serializable ) )
> + if ( ( valueSerializer != null ) && !( valueSerializer instanceof Serializable ) )
> {
> throw new IllegalArgumentException( I18n.err( I18n.ERR_521 ) );
> }
IMO this doesn't solves the problem doing the instanceOf check. The
method params are declared as jdbm.helper.Serializer which extends
Serializable. Therefore keySerializer and valueSerializer must be
instance of Serializable in the if restriction. If they can be
instanceOf anything else the method footprint needs to be changed for
not getting an IllegalArgument exception?
>
>
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.15 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAkwNEnAACgkQ2lZVCB08qHEj6ACgnS2X2ftiyvJAe4ADWxWxhf+j
ubcAoJ7nRUC+lf/XCIW0feX3McFKo6jZ
=2KCh
-----END PGP SIGNATURE-----
Re: svn commit: r952265 - /directory/apacheds/trunk/jdbm/src/main/java/jdbm/btree/BTree.java
Posted by Emmanuel Lecharny <el...@gmail.com>.
On 6/7/10 6:02 PM, Felix Knecht wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 06/07/10 17:51, Felix Knecht wrote:
>
>>
>>> Yes, this is the other option : not checking the keySerializer at all,
>>> as the test always evaluate to true. But certainly not checking half of
>>> the test (ie if ( keySerilizer != null ) then throw Exception),
>>> otherwise, trust me, you'll get tens of failing tests !!
>>>
> Either we throw an Exception when they are null or we can drop the 2 if
> classes, do you agree?
>
Atm, I don't know if it's OK to have null serializers... Anyway, it's
not tested, so let's drop the if tests.
--
Regards,
Cordialement,
Emmanuel Lécharny
www.nextury.com
Re: svn commit: r952265 - /directory/apacheds/trunk/jdbm/src/main/java/jdbm/btree/BTree.java
Posted by Felix Knecht <fe...@apache.org>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 06/07/10 17:51, Felix Knecht wrote:
>
>> Yes, this is the other option : not checking the keySerializer at all,
>> as the test always evaluate to true. But certainly not checking half of
>> the test (ie if ( keySerilizer != null ) then throw Exception),
>> otherwise, trust me, you'll get tens of failing tests !!
Either we throw an Exception when they are null or we can drop the 2 if
classes, do you agree?
Drop this
// This clause will never be true ...
if ( ( keySerializer != null ) && ! ( *true* ) )
{
throw new IllegalArgumentException( I18n.err( I18n.ERR_520 ) );
}
if ( ( valueSerializer != null ) && !( *true* ) )
{
throw new IllegalArgumentException( I18n.err( I18n.ERR_521 ) );
}
Throw exception when null
if ( ( keySerializer == null ) )
{
throw new IllegalArgumentException( I18n.err( I18n.ERR_520 ) );
}
if ( ( valueSerializer == null ) )
{
throw new IllegalArgumentException( I18n.err( I18n.ERR_521 ) );
}
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.15 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAkwNGBAACgkQ2lZVCB08qHEIKwCfYfc/YH8s4/nUBTtOEUCDIpy5
+YAAn2wt+7RWHaYNeFptaUxOF6GZfQ+p
=e4EF
-----END PGP SIGNATURE-----
Re: svn commit: r952265 - /directory/apacheds/trunk/jdbm/src/main/java/jdbm/btree/BTree.java
Posted by Felix Knecht <fe...@apache.org>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
> Yes, this is the other option : not checking the keySerializer at all,
> as the test always evaluate to true. But certainly not checking half of
> the test (ie if ( keySerilizer != null ) then throw Exception),
> otherwise, trust me, you'll get tens of failing tests !!
F*** I overlooked the "!" in there and removed the wrong part ... Going
to fix it as it should be.
Sorry
Felix
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.15 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAkwNFXoACgkQ2lZVCB08qHH8mwCgu/5N6WB/nTmNeXaMX8O78E2w
zjAAoKiVTAmbXIJkNLnNMzswNrrtxdvb
=50uf
-----END PGP SIGNATURE-----
Re: svn commit: r952265 - /directory/apacheds/trunk/jdbm/src/main/java/jdbm/btree/BTree.java
Posted by Emmanuel Lecharny <el...@gmail.com>.
On 6/7/10 5:38 PM, Felix Knecht wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 06/07/10 17:20, elecharny@apache.org wrote:
>
>> Author: elecharny
>> Date: Mon Jun 7 15:20:46 2010
>> New Revision: 952265
>>
>> URL: http://svn.apache.org/viewvc?rev=952265&view=rev
>> Log:
>> Reverted a wrong check on the keySerializer.
>>
>> Modified:
>> directory/apacheds/trunk/jdbm/src/main/java/jdbm/btree/BTree.java
>>
>> Modified: directory/apacheds/trunk/jdbm/src/main/java/jdbm/btree/BTree.java
>> URL: http://svn.apache.org/viewvc/directory/apacheds/trunk/jdbm/src/main/java/jdbm/btree/BTree.java?rev=952265&r1=952264&r2=952265&view=diff
>> ==============================================================================
>> --- directory/apacheds/trunk/jdbm/src/main/java/jdbm/btree/BTree.java (original)
>> +++ directory/apacheds/trunk/jdbm/src/main/java/jdbm/btree/BTree.java Mon Jun 7 15:20:46 2010
>> @@ -55,13 +55,13 @@ import java.io.Serializable;
>> import java.util.Comparator;
>> import java.util.concurrent.atomic.AtomicInteger;
>>
>> -import org.apache.directory.server.i18n.I18n;
>> -
>> import jdbm.RecordManager;
>> import jdbm.helper.Serializer;
>> import jdbm.helper.Tuple;
>> import jdbm.helper.TupleBrowser;
>>
>> +import org.apache.directory.server.i18n.I18n;
>> +
>>
>> /**
>> * B+Tree persistent indexing data structure. B+Trees are optimized for
>> @@ -206,12 +206,12 @@ public class BTree<K, V> implements Exte
>> throw new IllegalArgumentException( I18n.err( I18n.ERR_519 ) );
>> }
>>
>> - if ( keySerializer != null )
>> + if ( ( keySerializer != null )&& ! ( keySerializer instanceof Serializable ) )
>> {
>> throw new IllegalArgumentException( I18n.err( I18n.ERR_520 ) );
>> }
>>
>> - if ( valueSerializer != null&& !( valueSerializer instanceof Serializable ) )
>> + if ( ( valueSerializer != null )&& !( valueSerializer instanceof Serializable ) )
>> {
>> throw new IllegalArgumentException( I18n.err( I18n.ERR_521 ) );
>> }
>>
> IMO this doesn't solves the problem doing the instanceOf check. The
> method params are declared as jdbm.helper.Serializer which extends
> Serializable. Therefore keySerializer and valueSerializer must be
> instance of Serializable in the if restriction. If they can be
> instanceOf anything else the method footprint needs to be changed for
> not getting an IllegalArgument exception?
>
Yes, this is the other option : not checking the keySerializer at all,
as the test always evaluate to true. But certainly not checking half of
the test (ie if ( keySerilizer != null ) then throw Exception),
otherwise, trust me, you'll get tens of failing tests !!
--
Regards,
Cordialement,
Emmanuel Lécharny
www.nextury.com