LANG-1402: Null/index safe get methods for ArrayUtils#336
Conversation
|
User code has to cast the return to make use of it; that is not ideal. |
|
I added your suggested method and refactored the other to use generics. |
| } | ||
|
|
||
| if(index < 0 ){ | ||
| index = 0; |
There was a problem hiding this comment.
Surely a negative index should return the default?
| } | ||
|
|
||
| /** | ||
| * Gets an element from the array if the array is non-null and appropriately long, otherwise returns the specified value |
There was a problem hiding this comment.
Sorry, mistaken copy-paste there. Feel free to use this as a tutorial for how NOT to contribute.
| /** | ||
| * Gets an element from the array if the array is non-null and appropriately long, otherwise returns the specified value | ||
| * @param <T> the component type of the array | ||
| * @param array the array holding the desired element |
| * @param <T> the component type of the array | ||
| * @param array the array holding the desired element | ||
| * @param index the index of the element in the array | ||
| * @return The element in the array at the index, or null if it is ill-formatted |
There was a problem hiding this comment.
What does ill-formatted mean?
Can also return null if the array element is present and null.
I don't think this method is sufficiently useful to be worth adding
| * @param array the array holding the desired element | ||
| * @param index the index of the element in the array | ||
| * @param defaultReturn the object to be returned if the array is null or shorter than the index | ||
| * @return The element in the array at the specified index, or the given argument if it is ill-formatted |
There was a problem hiding this comment.
ill-formatted?
Again, I don't see the point of this method.
It's a bit more flexible than the original method.
But it's still only useful for the case where you can choose a default value that does not appear in the array, and you still have to check whether the return value is the default or not.
There was a problem hiding this comment.
My thought process was something similar to this https://github.com/apache/commons-lang/blob/master/src/main/java/org/apache/commons/lang3/StringUtils.java#L7456-L7458
I would understand if Strings are understood well-enough to merit a true default value and generic arrays aren't, so as to justify removing the two-argument edition.
There was a problem hiding this comment.
The usefulness of these would be to reduce some clutter in everyday development. Checking for both null and then length can be a tedious situation for developers striving to get high unit-test coverage.
isArrayIndexValid would reduce the number of branches in a typical use case, whereas get is probably a bit simpler. I'm can't say this with any degree of certainty, but a correctly-placed null element probably requires yet another check after the fact. Something like:
if(isArrayIndexValid(array, 0) && array[0] != null)
|
@sebbASF Thanks for your patience and efforts to make this a worthy contribution. I have removed the other methods per your suggestion. |
5 participants
@chtompki
If this is deemed worthy or needs improvement - let me know.