Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IMap.executeOnKeys() does not support the empty set, and is inconsistent with getAll() #7631

Closed
stevebarham opened this issue Feb 29, 2016 · 2 comments

Comments

Projects
None yet
3 participants
@stevebarham
Copy link

commented Feb 29, 2016

IMap.executeOnKeys has a fairly irritating wrinkle in 3.6.1; if invoked with an empty set, it throws a misleading NullPointerException.

I can see from the current master branch that this has been replaced with a more accurate IllegalArgumentException, indicating that an empty collection is not supported:

if (keys.size() == 0) {
  throw new IllegalArgumentException(EMPTY_COLLECTION_IS_NOT_ALLOWED);
}

I think this is bad for the following reasons:

  1. Checking for .size() == 0 may be inefficient, vs. using .isEmpty(), depending on the incoming set
  2. Per many other methods, the expected result of running an entry processor over no keys is not an exception, but an empty map. I agree that passing a null set of keys here represents an error.
  3. The behaviour is inconsistent with other set-receiving methods in IMap, for example getAll():
if (CollectionUtil.isEmpty(keys)) {
  return Collections.emptyMap();
}

Switching this to behave more like getAll() seems like a simple fix - please let me know if you agree, and I will send you a PR.

@jerrinot jerrinot added the Team: Core label Feb 29, 2016

@ahmetmircik

This comment has been minimized.

Copy link
Member

commented Mar 1, 2016

@stevebarham makes sense

stevebarham pushed a commit to stevebarham/hazelcast that referenced this issue Mar 1, 2016

ahmetmircik added a commit that referenced this issue Mar 2, 2016

Merge pull request #7640 from stevebarham/master
Empty set fix for executeOnKeys() #7631

@jerrinot jerrinot added this to the 3.7 milestone Mar 2, 2016

@jerrinot

This comment has been minimized.

Copy link
Contributor

commented Mar 2, 2016

Fixed by #7640

@jerrinot jerrinot closed this Mar 2, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.