Mailing-List: contact api-help@directory.apache.org; run by ezmlm
Precedence: bulk
Reply-To: api@directory.apache.org
Received-SPF: pass (athena.apache.org: local policy)
MIME-version: 1.0
Content-transfer-encoding: 7BIT
Content-type: text/plain; CHARSET=US-ASCII; format=flowed
Date: Thu, 14 Jan 2010 22:34:54 +0100
From: Matthew Swift <Matthew.Swift@Sun.COM>
Subject: Re: [DN] Existing API review
In-reply-to: <4B4E3328.7070501@apache.org>
Sender: Matthew.Swift@Sun.COM
To: api@directory.apache.org
Message-id: <4B4F8DFE.3060809@sun.com>
References: <4B4C7948.3000503@gmail.com> <4B4DAD6E.4040204@sun.com>
 <4B4E3328.7070501@apache.org>
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.8pre)
 Gecko/20100113 Lightning/1.0b1 Shredder/3.0.2pre


On 13/01/10 21:55, Stefan Seelmann wrote:
> Matthew Swift wrote:
>> On 12/01/10 14:29, Emmanuel Lecharny wrote:
>>
>> Even if you decide that caching is not required then that's no reason 
>> to develop an API which prevents you from implementing one in future. 
>> Using a normal constructor prevents the use of a cache (or forces you 
>> to use the pimpl idiom). If you extend the API later by adding a 
>> valueOf method then no existing applications will be able to take 
>> advantage of the perf improvement unless they are modified to use the 
>> new static factory method.
>
> Matthew, you mentioned good reasons for factory methdods. I think it 
> is not a big deal to add a one or two factory methods.


rootDN/nullDN/emptyDN is an obvious candidate

valueOf(String) is the other and perhaps valueOf(ByteString/byte[])

I don't like having two ways to achieve the same goal, hence my dislike 
of the DN(String) constructor, especially when users of the method will 
not automatically inherit performance improvements in future releases.


>
>>> The base constructor we can have are probably something like:
>>> DN()
>>> DN(String dnStr)
>>> DN( RDN... rdns)
>>> DN( RDN rdn, DN parent)
>
> +1, and additional
>   DN(DN localName, DN parent)

Agreed.


>
>
> And let's add simple methods to get the parent of an DN. Every time I 
> use JNDI I have to write a test to see the result of
>   ldapName.getPrefix( ldapName.size() - 1 )
>
>> Also, I strongly believe that DNs and RDNs and AVAs should be 
>> immutable objects (as well as any other low level API type). What do 
>> you think?
>
> Yes, I agree.
>
>>
>> Also, on the subject of AVAs - we have the AVA type as an inner class 
>> in RDN. I'm not particularly happy with this this, but less happy 
>> with it being a standalone class since AVAs are only used in RDNs and 
>> may introduce confusion elsewhere. For example, filters also use 
>> attribute value assertions but these are not the same type of object 
>> as an AVA even if they have the same name. For example, AVAs (in 
>> RDNs) do not allow attribute options or matching rules to be specified.
>>
>> What do you think? Inner class or standalone?
>
> It could be an inner class, but should be visible and constructable 
> from outside. The use case I see is to create an DN or RDN by 
> specifying the attribute types and values, without having to deal with 
> escaped characters. We need such functionality in Directory Studio, 
> when defining the RDN of an entry or when renaming an entry. The user 
> for example just types "a+b" into the value field, we construct a new 
> AVA("cn", "a+b") and the AVA implementation should handle the escaping 
> to "cn=a\+b".
>
>

There's no need for an AVA class at all from a construction point of 
view. For example, you could have an RDN method RDN.addAVA(type, value). 
The stronger requirement for an AVA class comes when you need to access 
the AVAs in an RDN since you want to get two values at once - the 
attribute type and attribute value. It is possible to avoid having an 
AVA class altogether by having methods like:

    int getAVACount()
    AttributeType getAVAAttributeType(int index)
    AttributeValue getAVAAttributeValue(int index)
    void addAVA(AttributeType, AttributeValue)

One less class is a good thing IMO especially when it's potentially a 
source of confusion (with filters), but I'm not a big fan of the 
indexing and the inability to easily iterate over the AVAs. I'm pretty 
undecided on this one.

Matt