Re: Fwd: [JLDAP] Store X509 object programmatically

["Sunil Kumar" <Sunilk@novell.com>]

Steve,
  Thanks for correcting me. I just looked at the implementation using
HashMap and didn't paid that it implements attention to the
java.util.Set contract.  As suggested  by you the implemenation doesn't
look correct. I would suggest following changes to the current
implementation:

<         if( this.map.containskey(attribute.getName().toUpperCase()))
<             return false;
<         else{
<             this.map.put(attribute.getName().toUpperCase(),
<                          attribute);
<             return true;
<         }
---
>         return (this.map.put(attribute.getName().toUpperCase(),
>                              attribute) != null);

let me know your comments about it?

Regards,
-Sunil.

>>> Steve Sonntag 8/26/2003 8:37:59 PM >>>
Sunil,
 
I don't agree that the javadoc is incorrect.  I believe the
implementation
is incorrect. See comments inline: 
> > 
> > >>> Sunil Kumar 25-Aug-03 11:14:09 PM >>>
> > Hi Diego,
> > I have verified the added certificate in my configuration. I used
Novell
> > ConsoleOne to validate it and its showing that the added
certificate is a
> > valid one.
> > 
> > As far as the seocnd problem reported by you i.e AttributeSet.add 
> > (LDAPAttribute) returns false. I looked at it and found that the
> > Documentation in JavaDoc for attributeSet.add method is wrong. In
javaDoc
> > they have specifed that:
> > add() method will "return true if the attribute was added."
> > 
> > Whereas actually it should be:-
> > " return true if there was already a previous value(non-null) for
the
> > specified attribute in the attribute set ."
> > 
I am not sure where you are quoting from, but ...

LDAPAttributeSet implements the interface Set and thus must abide by
that
contract.  The Set.add method states:
 
Returns: true if this set did not already contain the specified
element. 
 
Or to put it another way, returns true if the element was added.  Set
further states:
Adds the specified element to this set if it is not already present 
...  If this set already
contains the specified element, the call leaves this set unchanged and
returns false.
 
So now we must ask the question: Is the implementation correct.
 
The implementation is NOT correct in two ways:
 
1) The return value is incorrect:  The code looks something like:
 
       return (this.map.put(name, attribute) != null);
This returns true if the attribute exists.  It should return false.
 
2) map.put replaces the value.  
 
Quoting from java.util.HashMap
"If the map previously contained a mapping for this key, the old value
is replaced."
 
If the value exists, the set should be left unchanged. This also
violates the contract for Set.add. 
The code is incorrect, it should be changed.

 
> > Also I have found one more wrong statement in the JavaDoc.They have
mentioed :
> > * Adds the specified attribute to this set if it is not already
present.
> > * <p>If an attribute with the same name already exists in the set
then the
> > * specified attribute will not be added.</p>
> > 
> > Whereas it should be:-
> > * Adds the specified attribute to this set if it is not already
present.
> > * <p>If an attribute with the same name already exists in the set
then the
> > * old specified value for the attribute will be replaced by the new
Value..</p>
> > 
 
Again as previously stated, the contract for Set.add mandates that
the Set not be changed if an attempt is made to add an element already
in the set.  In this case, only one Attribute with the given attribute
name
is allowed in the set.  LDAPAttributeSet.add should return false if 
an Attribute by that name already exists in the set and should leave
the Set
unchanged.
 
-Steve