Copilot commented on code in PR #2558:
URL: https://github.com/apache/groovy/pull/2558#discussion_r3316475966
##########
src/main/java/org/codehaus/groovy/runtime/ArrayGroovyMethods.java:
##########
@@ -10111,6 +10235,161 @@ public static <T> Set<T> toSet(T[] self) {
return DefaultGroovyMethods.toSet(Arrays.asList(self));
}
+
//--------------------------------------------------------------------------
+ // toImmutableSet
+
+ /**
+ * Converts this array to an immutable Set, with each unique element added
to the set.
+ * <pre class="language-groovy groovyTestCase">
+ * boolean[] array = [true, false, true]
+ * Set expected = [true, false]
+ * assert array.toImmutableSet() == expected
+ * </pre>
+ *
+ * @param self a boolean array
+ * @return An immutable set containing the unique contents of this array.
+ * @since 6.0.0
+ */
+ public static Set<Boolean> toImmutableSet(boolean[] self) {
+ return Set.copyOf(toSet(self));
+ }
Review Comment:
The primitive-array `toImmutableSet` overloads call
`Set.copyOf(toSet(self))`, which builds a mutable `Set` and then copies it
again. This adds avoidable allocations and work, and for empty arrays still
pays the cost of building an empty set before returning an empty immutable set.
Consider returning `Collections.emptySet()` for empty arrays and otherwise
wrapping the newly created set with `Collections.unmodifiableSet(...)` (no
second copy).
##########
src/main/java/org/codehaus/groovy/runtime/ArrayGroovyMethods.java:
##########
@@ -9961,6 +9961,130 @@ public static <T> List<T> toList(T[] self) {
return new ArrayList<>(Arrays.asList(self));
}
+
//--------------------------------------------------------------------------
+ // toImmutableList
+
+ /**
+ * Converts this array to an immutable List of the same size, with each
+ * element added to the list.
+ *
+ * @param self a boolean array
+ * @return An immutable list containing the contents of this array.
+ * @since 6.0.0
+ */
+ public static List<Boolean> toImmutableList(boolean[] self) {
+ return List.copyOf(toList(self));
+ }
Review Comment:
The primitive-array `toImmutableList` overloads call
`List.copyOf(toList(self))`, which always allocates an intermediate `ArrayList`
(inside `primitiveArrayToList`) and then copies again in `copyOf`. This is
unnecessary overhead, especially for empty arrays where it still allocates
before returning an empty immutable list. Consider short-circuiting empty
arrays to `Collections.emptyList()` and otherwise wrapping the freshly-created
list with `Collections.unmodifiableList(...)` (no second copy).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
