[ 
https://issues.apache.org/jira/browse/COLLECTIONS-604?focusedWorklogId=730078&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-730078
 ]

ASF GitHub Bot logged work on COLLECTIONS-604:
----------------------------------------------

                Author: ASF GitHub Bot
            Created on: 19/Feb/22 20:09
            Start Date: 19/Feb/22 20:09
    Worklog Time Spent: 10m 
      Work Description: Simulant87 commented on a change in pull request #279:
URL: 
https://github.com/apache/commons-collections/pull/279#discussion_r810532515



##########
File path: src/main/java/org/apache/commons/collections4/CollectionUtils.java
##########
@@ -419,16 +419,14 @@ private CollectionUtils() {}
      * cost of an additional space complexity O(n).
      * </p>
      *
-     * @param coll1  the first collection, must not be null
-     * @param coll2  the second collection, must not be null
      * @return {@code true} iff the intersection of the collections has the 
same cardinality
      *   as the set of unique elements from the second collection
-     * @throws NullPointerException if coll1 or coll2 is null
+     * @returns true if first and second collection is null
      * @since 4.0
      */
     public static boolean containsAll(final Collection<?> coll1, final 
Collection<?> coll2) {
-        Objects.requireNonNull(coll1, "coll1");
-        Objects.requireNonNull(coll2, "coll2");
+        if (coll1 == null && coll2 == null) return true;
+

Review comment:
       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.  




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Issue Time Tracking
-------------------

    Worklog Id:     (was: 730078)
    Time Spent: 20m  (was: 10m)

> More uniform safe-null methods in CollectionUtils
> -------------------------------------------------
>
>                 Key: COLLECTIONS-604
>                 URL: https://issues.apache.org/jira/browse/COLLECTIONS-604
>             Project: Commons Collections
>          Issue Type: Bug
>          Components: Collection
>    Affects Versions: 4.1
>            Reporter: Bruno P. Kinoshita
>            Assignee: Bruno P. Kinoshita
>            Priority: Minor
>         Attachments: COLLECTIONS-604.csv
>
>          Time Spent: 20m
>  Remaining Estimate: 0h
>
> Currently, there are 65 public methods in `CollectionUtils`. And 53 without 
> the deprecated ones. Out of these, 24 handle `null` arguments. The remaining 
> methods throw a `NullPointerException` (NPE) at some part of its code.
> The methods that handle nulls, throw NPE, or return empty columns, boolean 
> values, or just doesn't do anything.
> As a user of the API, I would expect a more uniform behaviour across the 
> methods of `CollectionUtils`. COLLECTIONS-600 address one of these methods.
> `removeAll` (2x) and `retainAll` (2x) both state that a NPE will be thrown if 
> either parameter is `null`. However, they never check if the values are null, 
> and instead allow the code to run until a NPE is thrown.
> And the following code shows that `isEmpty` and `isFull` behave differently 
> too.
> {code:java}
> Collection<String> c = null;
> System.out.println(CollectionUtils.isEmpty(c)); // return true
> System.out.println(CollectionUtils.isFull(c));  // throws a NPE
> {code}
> If I don't have to worry about `null`s with `#isEmpty`, I would expect the 
> same from its related-method `#isFull`.
> What would be a good approach for it? Define a behaviour to all methods? Or 
> leave as is, but add more documentation?
> There are a few methods that can easily be updated to check for `null` 
> values. Others would require a bit more thinking. An example if the method in 
> question for COLLECTIONS-600. It checks equality of collections, and when 
> both collections are `null`, it says that they are equals. Google Guava 
> [Iterables#elementsEqual|https://github.com/google/guava/blob/312aeb938bd35b5b7c8930e19ff5d1ca38e49424/guava/src/com/google/common/collect/Iterables.java#L232]
>  and 
> [Iterators#elementsEqual|https://github.com/google/guava/blob/312aeb938bd35b5b7c8930e19ff5d1ca38e49424/guava/src/com/google/common/collect/Iterators.java#L274]
>  do not check for null values, for what it is worth.



--
This message was sent by Atlassian Jira
(v8.20.1#820001)

Reply via email to