osdir.com

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [collections] Null-safe addAll(final Collection<C> collection, final C... elements) [WAS] CollectionUtils not null-safe


I'd be much more  happy it the method was added, as something like

safeAddAll

so you knew when you were using it, but i agree then your api starts to bloat pretty badly.


CollectionUtils.isEmpty checking for null makes sense. You are explicitly using the method to check for null as a condition.


addAll doesn't say that to me when i use it. One argument for your proposal is that there really is no reason to use Collections.addAll

over myRealCollection.addAll(x);, so perhaps adding this null check gives it value.


Anyway, just my two cents. I have never used this method, so it doesn't effect me, so if in the end this is what you want to do, i guess it's ok, just showing the other side.


On 05/20/2018 11:02 AM, Gary Gregory wrote:
Shall we do this in a more pragmatic fashion? One method at a time. Let's
start with changing:

     /**
      * Adds all elements in the array to the given collection.
      *
      * @param <C>  the type of object the {@link Collection} contains
      * @param collection  the collection to add to, must not be null
      * @param elements  the array of elements to add, must not be null
      * @return {@code true} if the collection was changed, {@code false}
otherwise
      * @throws NullPointerException if the collection or array is null
      */
     public static <C> boolean addAll(final Collection<C> collection, final
C... elements) {
         boolean changed = false;
         for (final C element : elements) {
             changed |= collection.add(element);
         }
         return changed;
     }

to:

     /**
      * Adds all elements in the array to the given collection.
      *
      * @param <C>  the type of object the {@link Collection} contains
      * @param collection  the collection to add to
      * @param elements  the array of elements to add
      * @return {@code true} if the collection was changed, {@code false}
otherwise
      */
     public static <C> boolean addAll(final Collection<C> collection, final
C... elements) {
         boolean changed = false;
         if (collection != null && elements != null) {
             for (final C element : elements) {
                 changed |= collection.add(element);
             }
         }
         return changed;
     }

Which makes the method not blow up when either inputs is null.

Gary

On Sat, May 19, 2018 at 10:15 PM, Dave Brosius <dbrosius@xxxxxxxxxxxxxxx>
wrote:

This protection concept sounds perfectly rational, until you think of
other obvious things that do similar things to protect the coder.

myLong.equals(myString) doesn't throw an exception for instance, it just
return false.

but then you never know that you have a problem in your code and just
blindly go along using it seemingly working fine.

Similarly, given that collection.addAll(null) throws, papering over that
fact with CollectionUtils might do more harm than good.




On 05/19/2018 09:10 PM, Gary Gregory wrote:

On Sat, May 19, 2018 at 4:47 AM, sebb <sebbaz@xxxxxxxxx> wrote:

On 18 May 2018 at 20:34, Gary Gregory <garydgregory@xxxxxxxxx> wrote:
Hi All:

A lot of methods in CollectionUtils are not null-safe and are documented

as

such in Javadoc with throwing NPEs.

I'd like to change that.

To what?

For example, this should not cause an NPE:
collectionA = new ArrayList();
CollectionUtils.addAll(collectionA, (Integer[]) null);

Gary

The change is behavioral and BC would be preserved.
Thoughts?

It depends on the method whether a null parameter makes sense or not.
In some cases it may be confusing if null is accepted.
And what is meant by a null parameter.

In any case, any such changes need a big warning in the release notes.

Gary
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@xxxxxxxxxxxxxxxxxx
For additional commands, e-mail: dev-help@xxxxxxxxxxxxxxxxxx



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@xxxxxxxxxxxxxxxxxx
For additional commands, e-mail: dev-help@xxxxxxxxxxxxxxxxxx




---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@xxxxxxxxxxxxxxxxxx
For additional commands, e-mail: dev-help@xxxxxxxxxxxxxxxxxx