git.net

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

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


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
>
>


( ! ) Warning: include(msgfooter.php): failed to open stream: No such file or directory in /var/www/git/apache-commons-developers/msg06343.html on line 183
Call Stack
#TimeMemoryFunctionLocation
10.0009368856{main}( ).../msg06343.html:0

( ! ) Warning: include(): Failed opening 'msgfooter.php' for inclusion (include_path='.:/var/www/git') in /var/www/git/apache-commons-developers/msg06343.html on line 183
Call Stack
#TimeMemoryFunctionLocation
10.0009368856{main}( ).../msg06343.html:0