You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-dev@jackrabbit.apache.org by "reschke (via GitHub)" <gi...@apache.org> on 2024/09/05 13:26:02 UTC
[PR] OAK-11073: add Set.of(varargs) [jackrabbit-oak]
reschke opened a new pull request, #1690:
URL: https://github.com/apache/jackrabbit-oak/pull/1690
(no comment)
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-11073: add Set.of(varargs) and stream support [jackrabbit-oak]
Posted by "reschke (via GitHub)" <gi...@apache.org>.
reschke commented on code in PR #1690:
URL: https://github.com/apache/jackrabbit-oak/pull/1690#discussion_r1750066583
##########
oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/collections/CollectionUtils.java:
##########
@@ -75,10 +86,87 @@ public static <T> Set<T> toSet(final Iterable<T> iterable) {
* @return the set
* @param <T> the type of the elements
*/
- public static <T> Set<T> toSet(final Iterator<T> iterator) {
+ @NotNull
+ public static <T> Set<T> toSet(@NotNull final Iterator<T> iterator) {
+ Objects.requireNonNull(iterator);
final Set<T> result = new HashSet<>();
iterator.forEachRemaining(result::add);
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
+ */
+ @SafeVarargs
+ @NotNull
+ public static <T> Set<T> toSet(@NotNull final T... elements) {
+ Objects.requireNonNull(elements);
+ // make sure the set does not need to be resized given the initial content
+ float loadFactor = (float) 0.75; // HashSet default
+ int initialCapacity = 1+ (int) (elements.length / loadFactor);
+ final Set<T> result = new HashSet<>(initialCapacity, loadFactor);
+ for (T element : elements) {
+ result.add(element);
+ }
+ return result;
+ }
+
+ /**
+ * Convert an {@code Iterator} to an {@code Iterable}.
+ *
+ * @param iterator
+ * iterator to convert
+ * @return a single-use iterable for the iterator (representing the remaining
+ * elements in the iterator)
+ * @throws IllegalStateException
+ * when {@linkplain Iterable#iterator()} is called more than
+ * once
+ */
+ @NotNull
+ public static <T> Iterable<T> toIterable(@NotNull final Iterator<T> iterator) {
+ Objects.requireNonNull(iterator);
+
+ Iterable<T> delegate = new Iterable<T>() {
+
+ private boolean consumed = false;
+
+ @Override
+ public Iterator<T> iterator() {
+ if (consumed) {
+ throw new IllegalStateException("Iterator already returned once");
+ } else {
+ consumed = true;
+ return iterator;
+ }
+ }
+ };
Review Comment:
will do
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-11073: add Set.of(varargs) [jackrabbit-oak]
Posted by "nfsantos (via GitHub)" <gi...@apache.org>.
nfsantos commented on code in PR #1690:
URL: https://github.com/apache/jackrabbit-oak/pull/1690#discussion_r1749916391
##########
oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/collections/CollectionUtils.java:
##########
@@ -81,4 +83,69 @@ 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
+ */
+ @SafeVarargs
+ public static <T> Set<T> toSet(final T... elements) {
+ final Set<T> result = new HashSet<>(elements.length);
Review Comment:
Here you are specifying the `capacity` of the set, which in case of HashSet and HashMap works in a different way than arrays. The HashSet will ensure that `numberOfElements < capacity*loadFactor`. So setting the capacity to the number of elements that will be inserted here, guarantees that there will be a resizing operation. So it is probably better to set capacity to something like `elements.length * 1.25` (the default load factor is 0.75)
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-11073: add Set.of(varargs) and stream support [jackrabbit-oak]
Posted by "nfsantos (via GitHub)" <gi...@apache.org>.
nfsantos commented on code in PR #1690:
URL: https://github.com/apache/jackrabbit-oak/pull/1690#discussion_r1750060585
##########
oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/collections/CollectionUtils.java:
##########
@@ -75,10 +86,87 @@ public static <T> Set<T> toSet(final Iterable<T> iterable) {
* @return the set
* @param <T> the type of the elements
*/
- public static <T> Set<T> toSet(final Iterator<T> iterator) {
+ @NotNull
+ public static <T> Set<T> toSet(@NotNull final Iterator<T> iterator) {
+ Objects.requireNonNull(iterator);
final Set<T> result = new HashSet<>();
iterator.forEachRemaining(result::add);
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
+ */
+ @SafeVarargs
+ @NotNull
+ public static <T> Set<T> toSet(@NotNull final T... elements) {
+ Objects.requireNonNull(elements);
+ // make sure the set does not need to be resized given the initial content
+ float loadFactor = (float) 0.75; // HashSet default
+ int initialCapacity = 1+ (int) (elements.length / loadFactor);
Review Comment:
```suggestion
int initialCapacity = 1 + (int) (elements.length / loadFactor);
```
##########
oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/collections/CollectionUtils.java:
##########
@@ -75,10 +86,87 @@ public static <T> Set<T> toSet(final Iterable<T> iterable) {
* @return the set
* @param <T> the type of the elements
*/
- public static <T> Set<T> toSet(final Iterator<T> iterator) {
+ @NotNull
+ public static <T> Set<T> toSet(@NotNull final Iterator<T> iterator) {
+ Objects.requireNonNull(iterator);
final Set<T> result = new HashSet<>();
iterator.forEachRemaining(result::add);
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
+ */
+ @SafeVarargs
+ @NotNull
+ public static <T> Set<T> toSet(@NotNull final T... elements) {
+ Objects.requireNonNull(elements);
+ // make sure the set does not need to be resized given the initial content
+ float loadFactor = (float) 0.75; // HashSet default
+ int initialCapacity = 1+ (int) (elements.length / loadFactor);
+ final Set<T> result = new HashSet<>(initialCapacity, loadFactor);
+ for (T element : elements) {
+ result.add(element);
+ }
+ return result;
+ }
+
+ /**
+ * Convert an {@code Iterator} to an {@code Iterable}.
+ *
+ * @param iterator
+ * iterator to convert
+ * @return a single-use iterable for the iterator (representing the remaining
+ * elements in the iterator)
+ * @throws IllegalStateException
+ * when {@linkplain Iterable#iterator()} is called more than
+ * once
+ */
+ @NotNull
+ public static <T> Iterable<T> toIterable(@NotNull final Iterator<T> iterator) {
+ Objects.requireNonNull(iterator);
+
+ Iterable<T> delegate = new Iterable<T>() {
+
+ private boolean consumed = false;
+
+ @Override
+ public Iterator<T> iterator() {
+ if (consumed) {
+ throw new IllegalStateException("Iterator already returned once");
+ } else {
+ consumed = true;
+ return iterator;
+ }
+ }
+ };
Review Comment:
This is not thread safe, it can be consumed two or more times under concurrent access. If we don't want to make it thread safe, would be better to mention it in the Java docs.
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-11073: add Set.of(varargs) and stream support [jackrabbit-oak]
Posted by "nfsantos (via GitHub)" <gi...@apache.org>.
nfsantos commented on code in PR #1690:
URL: https://github.com/apache/jackrabbit-oak/pull/1690#discussion_r1750084935
##########
oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/collections/CollectionUtils.java:
##########
@@ -75,10 +86,91 @@ public static <T> Set<T> toSet(final Iterable<T> iterable) {
* @return the set
* @param <T> the type of the elements
*/
- public static <T> Set<T> toSet(final Iterator<T> iterator) {
+ @NotNull
+ public static <T> Set<T> toSet(@NotNull final Iterator<T> iterator) {
+ Objects.requireNonNull(iterator);
final Set<T> result = new HashSet<>();
iterator.forEachRemaining(result::add);
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
+ */
+ @SafeVarargs
+ @NotNull
+ public static <T> Set<T> toSet(@NotNull final T... elements) {
+ Objects.requireNonNull(elements);
+ // make sure the set does not need to be resized given the initial content
+ float loadFactor = (float) 0.75; // HashSet default
+ int initialCapacity = 1 + (int) (elements.length / loadFactor);
+ final Set<T> result = new HashSet<>(initialCapacity, loadFactor);
+ for (T element : elements) {
+ result.add(element);
+ }
+ return result;
+ }
+
+ /**
+ * Convert an {@code Iterator} to an {@code Iterable}.
+ * <p>
+ * This method is not thread-safe
Review Comment:
This is not accurate. The method is thread-safe, it can be called concurrently without any problems. But the Iterable returned is not thread safe. It's a really minor issue.
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-11073: add Set.of(varargs) and stream support [jackrabbit-oak]
Posted by "nfsantos (via GitHub)" <gi...@apache.org>.
nfsantos commented on code in PR #1690:
URL: https://github.com/apache/jackrabbit-oak/pull/1690#discussion_r1749959652
##########
oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/collections/CollectionUtils.java:
##########
@@ -81,4 +83,69 @@ 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
+ */
+ @SafeVarargs
+ public static <T> Set<T> toSet(final T... elements) {
+ final Set<T> result = new HashSet<>(elements.length);
Review Comment:
I'm not sure about the performance implications of using load factor of 1, I guess it will increase collisions which may degrade performance, of both inserts and lookups. As the returned map is mutable, we can expect that it will modified, so I think it's safer to keep the default load factor, which I guess was carefully chosen by very clever people to provide good performance. If we create the map with load factor of 1 and then it is used for a performance critical operation, we risk that the map will perform more poorly than if it had been created with the default load factor.
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-11073: add Set.of(varargs) [jackrabbit-oak]
Posted by "thomasmueller (via GitHub)" <gi...@apache.org>.
thomasmueller commented on code in PR #1690:
URL: https://github.com/apache/jackrabbit-oak/pull/1690#discussion_r1746574224
##########
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:
Doesn't this test case basically call the same method twice? If we change it to return an empty set, then the test won't fail
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-11073: add Set.of(varargs) [jackrabbit-oak]
Posted by "reschke (via GitHub)" <gi...@apache.org>.
reschke commented on code in PR #1690:
URL: https://github.com/apache/jackrabbit-oak/pull/1690#discussion_r1747561226
##########
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:
Should we specify the size for the HashSet?
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-11073: add Set.of(varargs) [jackrabbit-oak]
Posted by "rishabhdaim (via GitHub)" <gi...@apache.org>.
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-11073: add Set.of(varargs) and stream support [jackrabbit-oak]
Posted by "reschke (via GitHub)" <gi...@apache.org>.
reschke commented on code in PR #1690:
URL: https://github.com/apache/jackrabbit-oak/pull/1690#discussion_r1749947180
##########
oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/collections/CollectionUtils.java:
##########
@@ -81,4 +83,69 @@ 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
+ */
+ @SafeVarargs
+ public static <T> Set<T> toSet(final T... elements) {
+ final Set<T> result = new HashSet<>(elements.length);
Review Comment:
Let's just get rid of the argument.
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-11073: add Set.of(varargs) and stream support [jackrabbit-oak]
Posted by "reschke (via GitHub)" <gi...@apache.org>.
reschke merged PR #1690:
URL: https://github.com/apache/jackrabbit-oak/pull/1690
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-11073: add Set.of(varargs) and stream support [jackrabbit-oak]
Posted by "reschke (via GitHub)" <gi...@apache.org>.
reschke commented on code in PR #1690:
URL: https://github.com/apache/jackrabbit-oak/pull/1690#discussion_r1749947180
##########
oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/collections/CollectionUtils.java:
##########
@@ -81,4 +83,69 @@ 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
+ */
+ @SafeVarargs
+ public static <T> Set<T> toSet(final T... elements) {
+ final Set<T> result = new HashSet<>(elements.length);
Review Comment:
or specify the load factor as 1?
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-11073: add Set.of(varargs) [jackrabbit-oak]
Posted by "nfsantos (via GitHub)" <gi...@apache.org>.
nfsantos commented on code in PR #1690:
URL: https://github.com/apache/jackrabbit-oak/pull/1690#discussion_r1749916391
##########
oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/collections/CollectionUtils.java:
##########
@@ -81,4 +83,69 @@ 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
+ */
+ @SafeVarargs
+ public static <T> Set<T> toSet(final T... elements) {
+ final Set<T> result = new HashSet<>(elements.length);
Review Comment:
Here you are specifying the `capacity` of the set, which in case of HashSet and HashMap works in a different way than arrays. The HashSet will ensure that `numberOfElements < capacity*loadFactor`. So setting the capacity to the number of elements that will be inserted here, guarantees that there will be a resizing operation. So probably use a capacity of `elements.length * 1.25` (the default load factor is 0.75)
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-11073: add Set.of(varargs) [jackrabbit-oak]
Posted by "thomasmueller (via GitHub)" <gi...@apache.org>.
thomasmueller commented on code in PR #1690:
URL: https://github.com/apache/jackrabbit-oak/pull/1690#discussion_r1746574224
##########
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:
That test case will will call the same method twice... If we change it to return an empty set, then the test won't fail
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-11073: add Set.of(varargs) [jackrabbit-oak]
Posted by "reschke (via GitHub)" <gi...@apache.org>.
reschke commented on PR #1690:
URL: https://github.com/apache/jackrabbit-oak/pull/1690#issuecomment-2334614637
https://github.com/apache/jackrabbit-oak/pull/1690/files#r1747561226
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-11073: add Set.of(varargs) [jackrabbit-oak]
Posted by "reschke (via GitHub)" <gi...@apache.org>.
reschke commented on code in PR #1690:
URL: https://github.com/apache/jackrabbit-oak/pull/1690#discussion_r1749890736
##########
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:
Well, it'll be a NPE. I actually do not like the additional checks; they provide zero additional information, unless they come with a message.
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-11073: add Set.of(varargs) [jackrabbit-oak]
Posted by "reschke (via GitHub)" <gi...@apache.org>.
reschke commented on code in PR #1690:
URL: https://github.com/apache/jackrabbit-oak/pull/1690#discussion_r1749890736
##########
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:
Well, it'll be a NPE. I actually do not like the additional checks; they provide zero additional information, unless they come with a message.
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-11073: add Set.of(varargs) [jackrabbit-oak]
Posted by "reschke (via GitHub)" <gi...@apache.org>.
reschke commented on code in PR #1690:
URL: https://github.com/apache/jackrabbit-oak/pull/1690#discussion_r1749910754
##########
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:
is an empty set really desirable? Shouldn't we throw an NPE in both cases?
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org