directory-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alex Karasulu <akaras...@apache.org>
Subject Re: Some inconsistencies in the DN class
Date Sun, 13 Feb 2011 12:52:00 GMT
On Sun, Feb 13, 2011 at 1:23 PM, Emmanuel Lecharny <elecharny@gmail.com> wrote:
> Hi guys,
>
> as I was writing some doco for the LDAP-API, I checked the DN class and
> found some inconsistencies in the methods. For instance, we have various
> constructors, that can be split in two sets :
> - the Schema Aware DN constructors
> - the Schema Agnostic DN constructors
>
> 1) Schema Aware constructors :
> Dn(SchemaManager)
> Dn(Dn, SchemaManager)
> Dn(SchemaManager, String...)
> Dn(String, SchemaManager)
>
> 1) Schema Agnostic constructors :
> Dn()
> Dn(Dn)
> Dn(Rdn)
> Dn(String)
> *Dn(String, String, byte[])*  --> This constructor is only used by the
> deserialization method
> Dn(String, String, byte[], List<Rdn>)
> Dn(String...)
>
> As we can see, in the first set, we don't have the SchemaManager parameter
> put always at the same position in the arguments list. I would suggest that
> the Dn(Dn, SchemaManager) and Dn(String, SchemaManager) constructors be
> replaced by
> Dn(SchemaManager, Dn) and Dn(SchemaManager, String).

Big +1

> That raise a second issue : the Dn(SchemaManager, String...) and
> Dn(SchemaManager, String) constructors will collide, but there is no reason
> to have both. I suggest keeping only the Dn(SchemaManager, String...)
> method.

+1

> For the exact same reason, I would remove the DN(String) constructor to
> favor the DN(String...) one.

What does Dn(String...) with the array mean? On first sight I cannot
make sense of it. OK so you have an array of string arguments. What do
they represent?

Whereas it's clear what Dn(String) is. I know it's a Dn represented as
a String. The array of String arguments makes me ask ... Are these
Rdns? See the confusion?

> The Dn(String, String, byte[], List<Rdn>) constructor is only used inside
> the server, and I don't think we really need it. I suggest to remove it.
>
> The package protected constructor Dn(String, String, byte[]) is also an
> issue. There is no reason to expose it to the end user.
>
> Last, not least, in order to have some symmetry between both classes of
> constructor, I'd like to suggest we create a schema aware constructor :
> Dn(SchemaManager, Rdn...)

Is Rdn not schema aware already? Or is that just Dn?

> and to transform the Dn(Rdn) constructor to Dn(Rdn...).

+1

Alex

Mime
View raw message