You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by MarkDacek <gi...@git.apache.org> on 2018/07/07 16:47:09 UTC
[GitHub] commons-lang pull request #336: LANG-1402: Null/index safe get methods for A...
GitHub user MarkDacek opened a pull request:
https://github.com/apache/commons-lang/pull/336
LANG-1402: Null/index safe get methods for ArrayUtils
@chtompki
If this is deemed worthy or needs improvement - let me know.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/MarkDacek/commons-lang LANG-1402
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/commons-lang/pull/336.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #336
----
commit 6dacc2a421790770657f6e1f7ca8d6ec2e7f3a82
Author: MarkDacek <ma...@...>
Date: 2017-04-02T01:19:01Z
Merge remote-tracking branch 'APACHE/master'
commit a99631af9e24c2489ea63656decf89f506274519
Author: MarkDacek <ma...@...>
Date: 2017-04-22T13:59:14Z
Merge remote-tracking branch 'APACHE/master'
commit 95530e0be9bf6ab70c627295517d018ad3280489
Author: MarkDacek <ma...@...>
Date: 2017-04-24T22:18:28Z
Merge remote-tracking branch 'APACHE/master'
commit f4430b63a1bebc74ccb8519474d56c0918c9aea4
Author: MarkDacek <ma...@...>
Date: 2017-06-19T01:23:56Z
Merge remote-tracking branch 'APACHE/master'
commit 7ee1c65dc88ef3d9a83271b67fc210e7520762e1
Author: MarkDacek <ma...@...>
Date: 2017-09-05T00:56:27Z
Merge remote-tracking branch 'APACHE/master'
commit ce79ff2c77a2bcdf77300d470f418c1645742c5d
Author: MarkDacek <ma...@...>
Date: 2017-09-24T16:00:22Z
Merge remote-tracking branch 'APACHE/master'
commit 622824a51cd4f6cfee86c2ad3029821c7f096a36
Author: MarkDacek <ma...@...>
Date: 2017-10-23T22:57:29Z
Merge remote-tracking branch 'APACHE/master'
commit 56ac7bb06a7ff91358ffa837ce5dbfe4ad551454
Author: MarkDacek <ma...@...>
Date: 2018-02-14T23:44:54Z
Merge remote-tracking branch 'APACHE/master'
commit 526d1999b62d521a6c7bd9e1c9d1207809032a2e
Author: MarkDacek <ma...@...>
Date: 2018-07-07T15:27:05Z
Merge remote-tracking branch 'APACHE/master'
commit 91e0050de89c1430dfe98e4e8423c9dd82449fd2
Author: MarkDacek <ma...@...>
Date: 2018-07-07T16:45:15Z
LANG-1402: added get methods to ArrayUtils
----
---
[GitHub] commons-lang pull request #336: LANG-1402: Null/index safe get methods for A...
Posted by MarkDacek <gi...@git.apache.org>.
Github user MarkDacek commented on a diff in the pull request:
https://github.com/apache/commons-lang/pull/336#discussion_r200857547
--- Diff: src/main/java/org/apache/commons/lang3/ArrayUtils.java ---
@@ -8672,4 +8672,53 @@ public static void shuffle(final double[] array, final Random random) {
swap(array, i - 1, random.nextInt(i), 1);
}
}
+
+ /**
+ * Gets an element from the array if the array is non-null and appropriately long, otherwise returns null
+ * @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
+ * @since 3.8
+ */
+ public static <T> T get(T[] array, int index){
+ return get(array, index, null);
+ }
+
+ /**
+ * 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 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
+ * @since 3.8
+ */
+ public static <T> T get(T[] array, int index, T defaultReturn){
+ if(getLength(array) == 0 || array.length <= index){
+ return defaultReturn;
+ }
+
+ if(index < 0 ){
+ index = 0;
+ }
+
+ return array[index];
+ }
+
+ /**
+ * Gets an element from the array if the array is non-null and appropriately long, otherwise returns the specified value
--- End diff --
Sorry, mistaken copy-paste there. Feel free to use this as a tutorial for how NOT to contribute.
---
[GitHub] commons-lang pull request #336: LANG-1402: Null/index safe get methods for A...
Posted by MarkDacek <gi...@git.apache.org>.
Github user MarkDacek commented on a diff in the pull request:
https://github.com/apache/commons-lang/pull/336#discussion_r200857900
--- Diff: src/main/java/org/apache/commons/lang3/ArrayUtils.java ---
@@ -8672,4 +8672,53 @@ public static void shuffle(final double[] array, final Random random) {
swap(array, i - 1, random.nextInt(i), 1);
}
}
+
+ /**
+ * Gets an element from the array if the array is non-null and appropriately long, otherwise returns null
+ * @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
+ * @since 3.8
+ */
+ public static <T> T get(T[] array, int index){
+ return get(array, index, null);
+ }
+
+ /**
+ * 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 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
--- End diff --
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.
---
[GitHub] commons-lang issue #336: LANG-1402: Null/index safe get methods for ArrayUti...
Posted by MarkDacek <gi...@git.apache.org>.
Github user MarkDacek commented on the issue:
https://github.com/apache/commons-lang/pull/336
@sebbASF Thanks for your patience and efforts to make this a worthy contribution. I have removed the other methods per your suggestion.
---
[GitHub] commons-lang pull request #336: LANG-1402: Null/index safe get methods for A...
Posted by sebbASF <gi...@git.apache.org>.
Github user sebbASF commented on a diff in the pull request:
https://github.com/apache/commons-lang/pull/336#discussion_r200857077
--- Diff: src/main/java/org/apache/commons/lang3/ArrayUtils.java ---
@@ -8672,4 +8672,53 @@ public static void shuffle(final double[] array, final Random random) {
swap(array, i - 1, random.nextInt(i), 1);
}
}
+
+ /**
+ * Gets an element from the array if the array is non-null and appropriately long, otherwise returns null
+ * @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
+ * @since 3.8
+ */
+ public static <T> T get(T[] array, int index){
+ return get(array, index, null);
+ }
+
+ /**
+ * 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 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
+ * @since 3.8
+ */
+ public static <T> T get(T[] array, int index, T defaultReturn){
+ if(getLength(array) == 0 || array.length <= index){
+ return defaultReturn;
+ }
+
+ if(index < 0 ){
+ index = 0;
+ }
+
+ return array[index];
+ }
+
+ /**
+ * 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
--- End diff --
May be null
---
[GitHub] commons-lang issue #336: LANG-1402: Null/index safe get methods for ArrayUti...
Posted by sebbASF <gi...@git.apache.org>.
Github user sebbASF commented on the issue:
https://github.com/apache/commons-lang/pull/336
User code has to cast the return to make use of it; that is not ideal.
---
[GitHub] commons-lang issue #336: LANG-1402: Null/index safe get methods for ArrayUti...
Posted by MarkDacek <gi...@git.apache.org>.
Github user MarkDacek commented on the issue:
https://github.com/apache/commons-lang/pull/336
I added your suggested method and refactored the other to use generics.
---
[GitHub] commons-lang pull request #336: LANG-1402: Null/index safe get methods for A...
Posted by sebbASF <gi...@git.apache.org>.
Github user sebbASF commented on a diff in the pull request:
https://github.com/apache/commons-lang/pull/336#discussion_r200857303
--- Diff: src/main/java/org/apache/commons/lang3/ArrayUtils.java ---
@@ -8672,4 +8672,53 @@ public static void shuffle(final double[] array, final Random random) {
swap(array, i - 1, random.nextInt(i), 1);
}
}
+
+ /**
+ * Gets an element from the array if the array is non-null and appropriately long, otherwise returns null
+ * @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
+ * @since 3.8
+ */
+ public static <T> T get(T[] array, int index){
+ return get(array, index, null);
+ }
+
+ /**
+ * 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 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
--- End diff --
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.
---
[GitHub] commons-lang pull request #336: LANG-1402: Null/index safe get methods for A...
Posted by MarkDacek <gi...@git.apache.org>.
Github user MarkDacek commented on a diff in the pull request:
https://github.com/apache/commons-lang/pull/336#discussion_r200857848
--- Diff: src/main/java/org/apache/commons/lang3/ArrayUtils.java ---
@@ -8672,4 +8672,53 @@ public static void shuffle(final double[] array, final Random random) {
swap(array, i - 1, random.nextInt(i), 1);
}
}
+
+ /**
+ * Gets an element from the array if the array is non-null and appropriately long, otherwise returns null
+ * @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
+ * @since 3.8
+ */
+ public static <T> T get(T[] array, int index){
+ return get(array, index, null);
+ }
+
+ /**
+ * 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 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
+ * @since 3.8
+ */
+ public static <T> T get(T[] array, int index, T defaultReturn){
+ if(getLength(array) == 0 || array.length <= index){
+ return defaultReturn;
+ }
+
+ if(index < 0 ){
+ index = 0;
--- End diff --
Fixed.
---
[GitHub] commons-lang pull request #336: LANG-1402: Null/index safe get methods for A...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/commons-lang/pull/336
---
[GitHub] commons-lang pull request #336: LANG-1402: Null/index safe get methods for A...
Posted by MarkDacek <gi...@git.apache.org>.
Github user MarkDacek commented on a diff in the pull request:
https://github.com/apache/commons-lang/pull/336#discussion_r200858239
--- Diff: src/main/java/org/apache/commons/lang3/ArrayUtils.java ---
@@ -8672,4 +8672,53 @@ public static void shuffle(final double[] array, final Random random) {
swap(array, i - 1, random.nextInt(i), 1);
}
}
+
+ /**
+ * Gets an element from the array if the array is non-null and appropriately long, otherwise returns null
+ * @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
+ * @since 3.8
+ */
+ public static <T> T get(T[] array, int index){
+ return get(array, index, null);
+ }
+
+ /**
+ * 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 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
--- End diff --
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)
---
[GitHub] commons-lang pull request #336: LANG-1402: Null/index safe get methods for A...
Posted by sebbASF <gi...@git.apache.org>.
Github user sebbASF commented on a diff in the pull request:
https://github.com/apache/commons-lang/pull/336#discussion_r200857058
--- Diff: src/main/java/org/apache/commons/lang3/ArrayUtils.java ---
@@ -8672,4 +8672,53 @@ public static void shuffle(final double[] array, final Random random) {
swap(array, i - 1, random.nextInt(i), 1);
}
}
+
+ /**
+ * Gets an element from the array if the array is non-null and appropriately long, otherwise returns null
+ * @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
+ * @since 3.8
+ */
+ public static <T> T get(T[] array, int index){
+ return get(array, index, null);
+ }
+
+ /**
+ * 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 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
+ * @since 3.8
+ */
+ public static <T> T get(T[] array, int index, T defaultReturn){
+ if(getLength(array) == 0 || array.length <= index){
+ return defaultReturn;
+ }
+
+ if(index < 0 ){
+ index = 0;
--- End diff --
Surely a negative index should return the default?
---
[GitHub] commons-lang pull request #336: LANG-1402: Null/index safe get methods for A...
Posted by sebbASF <gi...@git.apache.org>.
Github user sebbASF commented on a diff in the pull request:
https://github.com/apache/commons-lang/pull/336#discussion_r200857070
--- Diff: src/main/java/org/apache/commons/lang3/ArrayUtils.java ---
@@ -8672,4 +8672,53 @@ public static void shuffle(final double[] array, final Random random) {
swap(array, i - 1, random.nextInt(i), 1);
}
}
+
+ /**
+ * Gets an element from the array if the array is non-null and appropriately long, otherwise returns null
+ * @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
+ * @since 3.8
+ */
+ public static <T> T get(T[] array, int index){
+ return get(array, index, null);
+ }
+
+ /**
+ * 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 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
+ * @since 3.8
+ */
+ public static <T> T get(T[] array, int index, T defaultReturn){
+ if(getLength(array) == 0 || array.length <= index){
+ return defaultReturn;
+ }
+
+ if(index < 0 ){
+ index = 0;
+ }
+
+ return array[index];
+ }
+
+ /**
+ * Gets an element from the array if the array is non-null and appropriately long, otherwise returns the specified value
--- End diff --
Does not agree with code
---
[GitHub] commons-lang pull request #336: LANG-1402: Null/index safe get methods for A...
Posted by sebbASF <gi...@git.apache.org>.
Github user sebbASF commented on a diff in the pull request:
https://github.com/apache/commons-lang/pull/336#discussion_r200857105
--- Diff: src/main/java/org/apache/commons/lang3/ArrayUtils.java ---
@@ -8672,4 +8672,53 @@ public static void shuffle(final double[] array, final Random random) {
swap(array, i - 1, random.nextInt(i), 1);
}
}
+
+ /**
+ * Gets an element from the array if the array is non-null and appropriately long, otherwise returns null
+ * @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
--- End diff --
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
---
[GitHub] commons-lang issue #336: LANG-1402: Null/index safe get methods for ArrayUti...
Posted by coveralls <gi...@git.apache.org>.
Github user coveralls commented on the issue:
https://github.com/apache/commons-lang/pull/336
[](https://coveralls.io/builds/17878362)
Coverage increased (+0.007%) to 95.251% when pulling **91e0050de89c1430dfe98e4e8423c9dd82449fd2 on MarkDacek:LANG-1402** into **c241b096d32f9ece918466f3614c143e8679dac5 on apache:master**.
---