directory-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Emmanuel Lecharny (JIRA)" <>
Subject [jira] [Commented] (DIRAPI-165) Add a FilterBuillder
Date Wed, 28 Jan 2015 14:46:35 GMT


Emmanuel Lecharny commented on DIRAPI-165:

Regarding the Javadoc :
- for private methods, I usually don't add a lot of doco. I certainly don't document the arguments,
return and exception, although I sometime do it when it helps someone who didn't wrote the
code, if it's complex. I generally drop a single line of text explaining what the method dos.
- for classes (even private) : same thing, but I try to add a doco no matter what.
- for tests : do what you prefer. I don't add doco most of the time, excpet what I'm in a
good mood.

I totally missed the fact that most of the classes are package protected. Again, here, I do
add things like /* No qualifier */ in front of every class/method/field that is package protected,
for clarification.

Anyway, a minimum doco for those classes are cool to have, just in case, and for the giys
doing maintenance.

Last, not list, for methods implementing an interface, where the Javadoc is already present,
I use something like :

 * {@inheritDoc}

Regarding the Operator : The {{AssertionType}} class is quite an old one, and it's used for
multiple purpose (there is a smell of bad cumulative design here...).

I would favor the definition of a {{FilterOperator}} enum for OR, AND and NOT operators only,
but available and used everywhere in the code, instead of {{AssertionType}}.

Regarding escapting : if you don't escape, the server will simply reject the search in specific
case where you have chars like '*', '(', ')', etc in the value.  Well, in fact, it's likely
that the client API will not be able to produce a valid SearchRequest, which will be encoded
in BER. 

If a user want to write things like :

equals( "givenName", "emmanuel is a ***" );

that is fine, but behind the curtain, the *** part should be escaped.

Last, not least, the Substring. I thought a lot about the ordering of elements. Beside the
fact that we can't use 'final' (damn reserved keywords...), Java does not like ellipsis except
at the end of the args.

One way to put the 'any' part in the middle would be to use String[] instead :

substring( String attr, String initial, String[] any, String end )

and if the user does not want to use 'any', he/she would use :

substring( "cn", "Emm", null, "el" );

That coud be an option. (and it's probably a better solution)

Your proposal is interesting, but you have some missing use cases (I = initial, A = any, F
= final) :
* I'*' -> startsWith
* '*'F -> endsWith
* '*'(A'*')+ -> contains
* I'*'(A'*')+ -> ???
* I'*'F -> ???
* '*'(A'*')+F -> ???
* I'*'(A'*')+F -> substring


Thanks !

PS : I have a few modifciations not yet commited I'd like to discuss befoe pushing it. I'll
add some more comment here.

> Add a FilterBuillder
> --------------------
>                 Key: DIRAPI-165
>                 URL:
>             Project: Directory Client API
>          Issue Type: New Feature
>    Affects Versions: 1.0.0-M21
>            Reporter: lucas theisen
>            Priority: Minor
>             Fix For: 1.0.0-M29
> Looking for something Fluent, in the _spirit_ of Hibernate Criteria.  May not seem like
much, but can drastically reduce query syntax issues. Also, you would likely be using a StringBuilder
anyway, so its not much different. 

This message was sent by Atlassian JIRA

View raw message