[Date Prev][Date Next] [Chronological] [Thread] [Top]

Re: Fwd: [JLDAP] Store X509 object programmatically



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