I'm looking at the source code of this package.
What is supposed to be the contract of:
- PatterMatcher.getMatcher(String expression)
- Matcher.matches(String input)
In both cases I see absolutely no reason to accept a null argument.
As far as I can read implementation differs regarding the acceptance of
null. Some will deal with it and go the extra step to compare null to
null, some will blow up.
GlobPatternMatcher goes the extra work of not blowing up if the pattern
syntax is invalid (but logging a message) and will later if the compiled
pattern is null returns true if the input is null too. That's pretty
wild and you can be sure that it will do the wrong thing and no one will
notice.
So it's not consistent and needs to be fixed so that it is more clear.
Do you agree that:
- The GlobPattern MUST failed (just like the regexp one) if the pattern
syntax is invalid.
- The expression must not be null.
- The input must not be null
Don't bother to fix anything, I will be hopefully cleaning some code,
documenting it (javadoc) and writing testcases.
And I'd like to remove this getInstance that is really annoying too.
Bonus question: Matcher.isExact(). While I understand that it is to
figure if a match is an exact match (vs a partial match)... I failed to
see the reason that motivated this method.
If I want to be nasty I could write a regexp which is an exact match
while the matcher will return that is not an exact match (because in
practice you only know if a matcher is exact only after doing it). and
that's the same for the ExactOrRegexp which can do an exact match but it
says it is not. I see that the isExact() is actually only used once. For
install(). And I'm not exactly sure to understand what implication it
has (I could go deeply in the code but think a short explanation of the
rationale behind it is certainly more interesting )
Thanks !
-- stephane
