directory-api mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stefan Seelmann <seelm...@apache.org>
Subject Re: [Shared] API Design Questionnaire #1
Date Sat, 29 Jan 2011 21:38:26 GMT
On Fri, Jan 28, 2011 at 5:58 PM, Alex Karasulu <akarasulu@apache.org> wrote:
> Hi community,
>
> Now that we're coming close to finishing up the shared refactoring we have
> to make some choices. Not all these choices have major impacts but some
> might. In the past we could do what we liked and change our minds etc. Now
> with a 1.0 of the shared libraries as the future mother of all Java LDAP
> APIs we're going to have to live with our choices.
>
> To opine, just place an 'X' in an option [  ] box.
>
>
> (1) ModifyRequest has a bunch of methods that were recently added to perform
> the same operations that you use the Modification interface for. This is
> redundant in my opinion and adds more unnecessary surface area. We don't
> need it and don't need an optional path to do the same thing confusing our
> users.  I suggest removing them.
>
> [  ] Yes - get rid of extra optional methods
> [X ] No  - keep the extra optional methods
> [  ] --- - I don't care about this stuff

I think the extra methods makes it much easier to construct a
ModifyRequest, one line of code instead of three lines:

    ModifyRequest request = ...;
    request.replace("mail", "value1", "value2", "value3");
is equivalent to
    ModifyRequest request = ...;
    EntryAttribute attribute = new DefaultEntryAttribute(("mail",
"value1", "value2", "value3");
    Modification mod = new
DefaultModification(ModificationOperation.REPLACE, attribute);
    request.addModification(mod);

IMO those are very close to LDIF syntax, I like them.

But it makes sense to rename the methods:
- remove -> addRemoveModification
- add -> addAddModification
- replace -> addReplaceModification
Additional those methods should return the created Modification object.
At last we may remove the "addModification()" methods.


> (2) Interfaces verses simple/basic classes implementing them have been
> something I've swayed back and forth on. Here are the options but note I am
> just using AddRequest as an example.
>
> [ ] - (a)
>            interface                                 = *I*AddRequest
>            simple API exposed implementation         = AddRequest
>            not so simple internal use implementation = AddRequest*Decoder*
> [  ] - (b)
>            interface                                 = AddRequest
>            simple API exposed implementation         = *Simple*AddRequest
>            not so simple internal use implementation = AddRequest*Decoder*
> [X] - (c)
>            interface                                 = AddRequest
>            simple API exposed implementation         = AddRequest*Impl*
>            not so simple internal use implementation = AddRequest*Decoder*
> [  ] - (d)
>            interface                                 = AddRequest
>            simple API exposed implementation         = *Basic*AddRequest
>            not so simple internal use implementation = AddRequest*Decoder*
>
> [  ] - (e) I pick the fat lady with the pink tutu ....
>
> We're applying option 'C' right now. I'm torn but think A might suite us
> better for the long term, and for any situation. You also know what's an
> interface and what's not although the IDE automatically shows you this stuff
> on the package/class browser.

This is my opinion for a low-level API, which 1:1 maps LDAP
terminology to the Java API. I think we should additional have a
simplified API where the user don't need to deal with request and
response objects at all.

BTW: We have this discussion again and again ;-) We really need to
decide a consistent naming.


> (3) JNDI remnants are somewhat still present even if we've gotten rid of
> most of them. In the model interfaces for Control, ExtendedRequest, and
> ExtendedResponse (IntermediateResponse as well but this has nothing to do
> with JNDI) we have exposed access to ASN.1 encoded data. I think this is a
> big mistake to do in the public API.
>
> Controls and extended operation interfaces should simply expose
> parameters/properties leaving the rest up to the CODEC to handle. There
> should be no need to get or set the entire ASN.1 blob for the control or
> extended operation's request response pair. What good does it do anyway?
> It's just opening the door for users to incorrectly alter properly encoded
> ASN.1 data causing problems. I think the getValue() setValue() methods
> remained after we ran screaming away from JNDI. But it seems these
> interfaces remained and now they're a liability. Where manipulation of the
> binary ASN.1 data is needed we can leave this up to the CODEC under a
> decorator to do.
>
> I recommend removing these, what do you think?
>
> [X] Yes - Remove them, they are more bad then good
> [  ] No  - Don't remove them, I like using em
> [  ] --- - I don't give a rat's a**

I see you already removed the methods, sorry for not responding in time...

Kind Regards,
Stefan

Mime
View raw message