Hi Jonathan, thanks for the feedback! See answers inline:
There's no explanation of the behavior anywhere. In the constructors and addPrefix() methods, you should document that this creates an OR condition across all of the prefixes, correct?
- good point, added some more explanations.
No need to instantiate a new comparator all the time (use Bytes.BYTES_COMPARATOR)
- Didn't know it existed. Changed.
Something seems odd when you keep adding to the end of a List and then sort. How about a TreeSet? You can easily ignore dupes that way.
- This is intentional. Sorting is done only during initialization but accessing a ArrayList, which is actually based on an array, is much more efficient than accessing a tree, so I sacrifice the aesthetics of the code for better runtime performance.
There's no input verification so, for example, you could pass a null to the constructor or an empty byte and have some strange behavior. Like it will instantiate okay but then you'll get server-side NPEs or IOOB.
- it's a good point but I've looked and no other filter is validating its input either. I can throw a InvalidArgumentException but don't know if it's a good idea considering it's not the norm.
this.prefixes.size() == 0 -> this.prefixes.isEmpty()
- ok, changed.
your comment at the top of filterColumn, i wouldn't exactly call it a workaround, but it's a good comment. looking at the logic, it seems like correct behavior would be that it can be called with current == size() but it would be a bug if current > size(), right? should you add an assert or throw an exception?
- well it is kind of a workaround, because as an individual filter I expect not be called again after returning NEXT_ROW, however, when used with FilterList the filter does get called again which puts it in an ilegal state, so it has to explicitly handle that case. That is also why it can't throw an exception in that scenario, because it seems to be happening normally when used with FilterList. as for "current" it has to be smaller than size() or it would be outside the bounds of the array.