Skip to content

COLLECTIONS-604#279

Closed
Kaammill wants to merge 2 commits intoapache:masterfrom
Kaammill:master
Closed

COLLECTIONS-604#279
Kaammill wants to merge 2 commits intoapache:masterfrom
Kaammill:master

Conversation

@Kaammill
Copy link

Hi,
I was thinking about the more uniform safe-null methods in CollectionUtils from jira task: COLLECTIONS-604. In my opinion it would be nice to have one way of treating null-safe methods. I decided to try wit the approach that the methods that already return Boolean values could be null-save. I would appreciate all kind of feedback on this task :)

Objects.requireNonNull(coll1, "coll1");
Objects.requireNonNull(coll2, "coll2");
if (coll1 == null && coll2 == null) return true;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code below will still throw a now no longer documented and expected NullPointerException if coll1 if not null but coll2 is null.
It's the same thing for the change in the method below, and possible more, I haven't read through the whole PR.

Copy link
Contributor

@Claudenw Claudenw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you need to think about set operations.

what does containsAny(x,y) mean if

  • y is null? If this is a set operation then all X contain the null set.
  • x is null? If this is a set operation then the null set does not contain any other set, except perhaps the null set.

I think the same questions need to be answered for all the collections questions.

I would recommend updating the javadoc for each method so that it is clear what the various null values return.

@Claudenw
Copy link
Contributor

@Kaammill The more I think about this change the more I think that there is a significant issue.
Changing the code to not throw an NPE when the NPE was documented in the earlier release is (I think) a breaking change and would require a new major version.

Alternatively, a new set of classes that handle Null pointer arguments differently might be valuable. However, this is a discussion to have on the dev mailing list.

Are you still interested in making these changes?

@garydgregory
Copy link
Member

Closing: No reply from OP in over a year.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

Comments