directory-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stefan Seelmann <seelm...@apache.org>
Subject Re: Some inconsistencies in the DN class
Date Sun, 13 Feb 2011 12:20:51 GMT
On Sun, Feb 13, 2011 at 12: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).

+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.
>
> For the exact same reason, I would remove the DN(String) constructor to
> favor the DN(String...) one.

+1

> 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.

+1, if possible

> The package protected constructor Dn(String, String, byte[]) is also an
> issue. There is no reason to expose it to the end user.

+1

> 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...)
> and to transform the Dn(Rdn) constructor to Dn(Rdn...).

+1

> Thoughts ?

The Dn(Dn) constructor, I think that is useless and should be removed.

So as a result we will have the following constructors, right?
  Dn(String...)
  Dn(Rdn...)
  Dn(SchemaManager, String...)
  Dn(SchemaManager, Rdn...)
  Dn(SchemaManager, Dn)

There is also an constructor I am missing: to create an DN  based on a
parent DN and the child RDN. For example
  Dn(Rdn child, Dn parent)
  Dn(SchemaManger, Rdn child, Dn parent)

Maybe an insane idea, but what about a all-you-can-eat constructor
  Dn(Object...)
  Dn(SchemaManger, Object...)
where the objects can be of type String, Dn, or Rdn? That would allow
maximal flexibility when constructing an DN. Sometimes you have the
parent as Dn object and the attribute type and value as string, then a
simple
  Dn parentDn = ...;
  new Dn(schemaManger, "ou", "groups", parentDn);
would work.

Kind Regards,
Stefan

Mime
View raw message