On Fri, Jan 28, 2011 at 5:58 PM, Alex Karasulu <[email protected]> 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
