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 :
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
PS : I have a few modifciations not yet commited I'd like to discuss befoe pushing it. I'll add some more comment here.