denis-chudov commented on code in PR #4707:
URL: https://github.com/apache/ignite-3/pull/4707#discussion_r1846159797
##########
modules/core/src/main/java/org/apache/ignite/internal/util/CollectionUtils.java:
##########
@@ -171,6 +171,33 @@ public static <T> Set<T> union(@Nullable Set<T> set,
@Nullable T... ts) {
return unmodifiableSet(res);
}
+
+ /**
+ * Logical union on two probably {@code null} or empty sets.
+ *
+ * @param firstSet First operand.
+ * @param secondSet Second operand.
+ * @return Result of the union.
+ */
+ public static <T> Set<T> union(@Nullable Set<T> firstSet, @Nullable Set<T>
secondSet) {
+ boolean isFirstSetEmptyOrNull = nullOrEmpty(firstSet);
+ boolean isSecondSetEmptyOrNull = nullOrEmpty(secondSet);
+
+ if (isFirstSetEmptyOrNull && isSecondSetEmptyOrNull) {
+ return new HashSet<>();
+ } else if (isFirstSetEmptyOrNull) {
+ return new HashSet<>(secondSet);
+ } else if (isSecondSetEmptyOrNull) {
+ return new HashSet<>(firstSet);
Review Comment:
why not just `firstSet`?
##########
modules/core/src/main/java/org/apache/ignite/internal/util/CollectionUtils.java:
##########
@@ -171,6 +171,33 @@ public static <T> Set<T> union(@Nullable Set<T> set,
@Nullable T... ts) {
return unmodifiableSet(res);
}
+
+ /**
+ * Logical union on two probably {@code null} or empty sets.
+ *
+ * @param firstSet First operand.
+ * @param secondSet Second operand.
+ * @return Result of the union.
+ */
+ public static <T> Set<T> union(@Nullable Set<T> firstSet, @Nullable Set<T>
secondSet) {
+ boolean isFirstSetEmptyOrNull = nullOrEmpty(firstSet);
+ boolean isSecondSetEmptyOrNull = nullOrEmpty(secondSet);
+
+ if (isFirstSetEmptyOrNull && isSecondSetEmptyOrNull) {
+ return new HashSet<>();
+ } else if (isFirstSetEmptyOrNull) {
+ return new HashSet<>(secondSet);
Review Comment:
I would suggest `Set.of()` here, it provides lightweight immutable
collection for zero element set.
##########
modules/core/src/main/java/org/apache/ignite/internal/util/CollectionUtils.java:
##########
@@ -147,28 +146,30 @@ public static boolean nullOrEmpty(@Nullable Iterator<?>
iter) {
}
/**
- * Union set and items.
+ * Logical union operation on two probably {@code null} or empty sets.
*
- * @param set Set.
- * @param ts Items.
- * @param <T> Type of the elements of set and items..
- * @return Immutable union of set and items.
+ * @param firstSet First operand.
+ * @param secondSet Second operand.
+ * @return Result of the union on two sets that equals to all unique
elements from the first and the second set or empty set if both
+ * given sets are empty.
*/
- @SafeVarargs
- public static <T> Set<T> union(@Nullable Set<T> set, @Nullable T... ts) {
- if (nullOrEmpty(set)) {
- return ts == null || ts.length == 0 ? Set.of() : Set.of(ts);
- }
-
- if (ts == null || ts.length == 0) {
- return unmodifiableSet(set);
+ public static <T> Set<T> union(@Nullable Set<T> firstSet, @Nullable Set<T>
secondSet) {
+ boolean isFirstSetEmptyOrNull = nullOrEmpty(firstSet);
+ boolean isSecondSetEmptyOrNull = nullOrEmpty(secondSet);
+
+ if (isFirstSetEmptyOrNull && isSecondSetEmptyOrNull) {
+ return new HashSet<>();
+ } else if (isFirstSetEmptyOrNull) {
+ return new HashSet<>(secondSet);
+ } else if (isSecondSetEmptyOrNull) {
+ return new HashSet<>(firstSet);
+ } else {
+ var union = new HashSet<>(firstSet);
+
+ union.addAll(secondSet);
+
+ return union;
Review Comment:
btw, taking into account two comments above, in order to provide consistent
output, maybe here also an ummutable set should be returned
--
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: notifications-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
