rishabhdaim commented on code in PR #1690:
URL: https://github.com/apache/jackrabbit-oak/pull/1690#discussion_r1749276817
##########
oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/collections/CollectionUtils.java:
##########
@@ -81,4 +81,17 @@ public static <T> Set<T> toSet(final Iterator<T> iterator) {
return result;
}
+ /**
+ * Convert a vararg list of items to a set. The returning set is mutable
and supports all optional operations.
+ * @param elements elements to convert
+ * @return the set
+ * @param <T> the type of the elements
+ */
+ public static <T> Set<T> toSet(@SuppressWarnings("unchecked") final T...
elements) {
+ final Set<T> result = new HashSet<>();
Review Comment:
We should also handle cases where `elements` are passed a `null`.
##########
oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/collections/CollectionUtilsTest.java:
##########
@@ -64,4 +64,10 @@ public void iteratorToSet() {
Assert.assertEquals(s, CollectionUtils.toSet(iterable.iterator()));
}
+
+ @Test
+ public void arrayToSet() {
+ final Set<String> s = CollectionUtils.toSet(data);
+ Assert.assertEquals(s, CollectionUtils.toSet(data.toArray()));
Review Comment:
Add a test case for the `null` element.
This throws a surprise result.
If we pass `null` to `toSet(T... elements)`, it gives a compilation error
due to an ambiguous method call since we have 2 other methods with the same
name.
On passing `null` after type casting to `Object` as `toSet((Object)null)`,
it returns a `set` with one `null` element.
`@Test
public void nullToSet() {
Assert.assertEquals(new HashSet<>(), CollectionUtils.toSet((Object)
null));
}`
The above fails whereas (below) if we typecast it to `Object[]` passes.
It returns the correct value of an empty set.
`@Test
public void nullToSet() {
Assert.assertEquals(new HashSet<>(),
CollectionUtils.toSet((Object[]) null));
}`
##########
oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/collections/CollectionUtils.java:
##########
@@ -81,4 +81,17 @@ public static <T> Set<T> toSet(final Iterator<T> iterator) {
return result;
}
+ /**
+ * Convert a vararg list of items to a set. The returning set is mutable
and supports all optional operations.
+ * @param elements elements to convert
+ * @return the set
+ * @param <T> the type of the elements
+ */
+ public static <T> Set<T> toSet(@SuppressWarnings("unchecked") final T...
elements) {
Review Comment:
We remove the redundant suppression for `unchecked` and should add
`@SafeVarargs` to method.
--
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]
