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

Issue #3742 Changing the axesSizeProduct entries to Integer.MAX_VALUE instead of throwing exception #3871

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

HelenaGregg
Copy link

#3742 said that they would like to be able to create a Sets::cartesianProduct object that would have more than 2^31-1 elements in the resulting set. Currently, it throws an IllegalArgumentException when this happens. @Meijuh said that since "java.util.Set::size() specifies that if the size would be more than Integer.MAX_VALUE it would simply be Integer.MAX_VALUE" this should be fixed.
So the code change is that instead of throwing an exception, the remaining lower parts of the axesSizeProduct integer array are set to Integer.MAX_VALUE so the size function (which returns axesSizeProduct[0]) will now return Integer.MAX_VALUE.

@netdpb netdpb added P3 package=collect type=enhancement Make an existing feature better labels Apr 22, 2020
@lowasser
Copy link
Contributor

Several other methods are defined in terms of axesSizeProduct and make assumptions about how it works, so please add tests for them; I suspect that several of them are broken in the current version.
In particular, iterator and contains. (Further thinking about this brought up #3873 and #3874 .)

In particular:

  • I'm not sure List.indexOf is well-defined for a list of size greater than Integer.MAX_VALUE, so we'd have to make a decision what to do there. I prefer throwing IndexOutOfBoundsException, if only because Objects.equal(list.get(list.indexOf(o)), o) should always be true if index is not -1, and list.indexOf(o) >= 0 should be as equivalent as possible to list.contains(o). (Note that this change also affects Lists.cartesianProduct, so indexOf is a method that needs to be supported.
  • The iterator needs to return the next element after you've called next Integer.MAX_VALUE times. Unfortunately, I'm not sure that can be tested without being very slow, and I don't know how to solve that.
  • contains definitely needs to be tested, including on elements whose "index" would be greater than Integer.MAX_VALUE. That probably means it needs a new implementation separate from indexOf, which would ideally also address Sets.cartesianProduct.contains' complexity depended on now-removed optimization in Android version #3873.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants