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 <dbros...@mebigfatguy.com>
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 <seb...@gmail.com> wrote:
On 18 May 2018 at 20:34, Gary Gregory <garydgreg...@gmail.com> 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-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
