accumulo-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <>
Subject [GitHub] ctubbsii commented on issue #901: Fixes #888 - AccumuloInputFormat.fetchColumns should accept empty col…
Date Tue, 15 Jan 2019 23:29:52 GMT
ctubbsii commented on issue #901: Fixes #888 - AccumuloInputFormat.fetchColumns should accept
empty col…
   Hi @jzgithub1 ; Upon reviewing this issue, I'm not sure this (#888) should be "fixed" at
all. The point of the issue was to address the "fluent" use case, to make it easier to do
things like:
     // INSTEAD OF:
     // if ((input = getUserInput()) != null) {
     //   builder.fetchColumns(input).store();
     // } else {
     // }
   private Collection<String> getUserInput() {
     // some method that can return an empty collection
   However, in doing so, it also permits this very very confusing behavior:
   where you are *literally* telling the API to fetch **NO** columns, and it is doing the
exact opposite from what you said. This isn't good semantics.
   I thought of two other suggestions for how this could be changed to support the fluent
case and have clear semantics:
     builder.fetchColumnsOrAll(getUserInput()).store(); // OR
   In these, it's a lot more clear that there's some "special behavior" in the case of `null`
or `Optional.empty()`, instead of literally doing the opposite of what the user asked the
API to do.
   However, it's not clear that either of these two suggestions are better than the `IllegalArgumentException`
that was there originally. Perhaps somebody else has a good suggestion for clear API semantics
*AND* that supports the fluent use case. If not, while I appreciate that you spent time on
it, I'm inclined to think that it's no longer a good idea.

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:

With regards,
Apache Git Services

View raw message