Merge pull request #1376 from TheJavaGuy/1372-optimize-withAll-on-bags

Optimize withAll for primitive bag factories
eclipse · Aug 23, 2022 · 5dd90aa · 5dd90aa
2 parents 684928d + 6c06d5f
commit 5dd90aa
Show file tree

Hide file tree

Showing 5 changed files with 110 additions and 4 deletions.
diff --git a/...code-generator/src/main/resources/impl/bag/immutable/immutablePrimitiveBagFactoryImpl.stg b/...code-generator/src/main/resources/impl/bag/immutable/immutablePrimitiveBagFactoryImpl.stg
@@ -15,7 +15,9 @@ package org.eclipse.collections.impl.bag.immutable.primitive;
 
 import org.eclipse.collections.api.<name>Iterable;
 import org.eclipse.collections.api.bag.primitive.Immutable<name>Bag;
+import org.eclipse.collections.api.bag.primitive.Mutable<name>Bag;
 import org.eclipse.collections.api.factory.bag.primitive.Immutable<name>BagFactory;
+import org.eclipse.collections.impl.bag.mutable.primitive.<name>HashBag;
 import org.eclipse.collections.impl.factory.primitive.<name>Bags;
 <(wideStreamImport.(type))>
 
@@ -92,7 +94,21 @@ public class Immutable<name>BagFactoryImpl implements Immutable<name>BagFactory
         {
             return (Immutable<name>Bag) items;
         }
-        return this.with(items.toArray());
+        if (items == null)
+        {
+            return this.with();
+        }
+        Mutable<name>Bag bag = new <name>HashBag();
+        items.forEach(bag::add);
+        if (bag.size() == 0)
+        {
+            return this.with();
+        }
+        if (bag.size() == 1)
+        {
+            return this.with(bag.toArray());
+        }
+        return Immutable<name>HashBag.newBagWith(bag);
     }
 
     /**
@@ -145,7 +161,21 @@ public Immutable<name>Bag ofAll(<(wideStream.(type))> items)
 @Override
 public Immutable<name>Bag withAll(<(wideStream.(type))> items)
 {
-    return this.with(items.toArray());
+    if (items == null)
+    {
+        return this.with();
+    }
+    Mutable<name>Bag bag = new <name>HashBag();
+    items.forEach(bag::add);
+    if (bag.size() == 0)
+    {
+        return this.with();
+    }
+    if (bag.size() == 1)
+    {
+        return this.with(bag.toArray());
+    }
+    return Immutable<name>HashBag.newBagWith(bag);
 }
 >>
 

diff --git a/...ctions-code-generator/src/main/resources/impl/bag/immutable/immutablePrimitiveHashBag.stg b/...ctions-code-generator/src/main/resources/impl/bag/immutable/immutablePrimitiveHashBag.stg
@@ -66,6 +66,13 @@ final class Immutable<name>HashBag implements Immutable<name>Bag, Serializable
         this.delegate = <name>HashBag.newBagWith(newElements);
     }
 
+    private Immutable<name>HashBag(Mutable<name>Bag delegate)
+    {
+        Mutable<name>Bag defensiveCopy = new <name>HashBag(delegate);
+        this.checkOptimizedSize(defensiveCopy.size());
+        this.delegate = defensiveCopy;
+    }
+
     private void checkOptimizedSize(int length)
     {
         if (length \<= 1)
@@ -79,6 +86,11 @@ final class Immutable<name>HashBag implements Immutable<name>Bag, Serializable
         return new Immutable<name>HashBag(elements);
     }
 
+    public static Immutable<name>HashBag newBagWith(Mutable<name>Bag elements)
+    {
+        return new Immutable<name>HashBag(elements);
+    }
+
     @Override
     public Immutable<name>Bag newWith(<type> element)
     {

diff --git a/...ons-code-generator/src/main/resources/impl/bag/mutable/mutablePrimitiveBagFactoryImpl.stg b/...ons-code-generator/src/main/resources/impl/bag/mutable/mutablePrimitiveBagFactoryImpl.stg
@@ -126,7 +126,9 @@ public Mutable<name>Bag ofAll(<(wideStream.(type))> items)
 @Override
 public Mutable<name>Bag withAll(<(wideStream.(type))> items)
 {
-    return this.with(items.toArray());
+    Mutable<name>Bag bag = new <name>HashBag();
+    items.forEach(bag::add);
+    return bag;
 }
 >>
 

diff --git a/...ns-code-generator/src/main/resources/test/bag/immutable/immutablePrimitiveHashBagTest.stg b/...ns-code-generator/src/main/resources/test/bag/immutable/immutablePrimitiveHashBagTest.stg
@@ -19,6 +19,8 @@ body(type, name, wrapperName) ::= <<
 package org.eclipse.collections.impl.bag.immutable.primitive;
 
 import org.eclipse.collections.api.bag.primitive.Immutable<name>Bag;
+import org.eclipse.collections.api.bag.primitive.Mutable<name>Bag;
+import org.eclipse.collections.impl.bag.mutable.primitive.<name>HashBag;
 import org.eclipse.collections.impl.factory.primitive.<name>Bags;
 import org.eclipse.collections.impl.math.Mutable<wrapperName>;
 import org.junit.Assert;
@@ -60,6 +62,21 @@ public class Immutable<name>HashBagTest extends AbstractImmutable<name>BagTestCa
         Immutable<name>Set actual = bag.selectUnique();
         Assert.assertEquals(expected, actual);
     }
+
+    @Test
+    public void newBagWithMutable()
+    {
+        <name>HashBag mutable3 = <name>HashBag.newBagWith(<["1", "2", "3"]:(literal.(type))(); separator=", ">);
+        Immutable<name>Bag immutable3 = mutable3.toImmutable();
+        <name>HashBag mutable4 = <name>HashBag.newBagWith(<["1", "2", "3", "4"]:(literal.(type))(); separator=", ">);
+        Mutable<name>Bag mutableBag = new <name>HashBag(<["1", "2", "3"]:(literal.(type))(); separator=", ">);
+        Immutable<name>HashBag immutableBag = Immutable<name>HashBag.newBagWith(mutableBag);
+        Assert.assertEquals(mutable3, mutableBag);
+        Assert.assertEquals(immutable3, immutableBag);
+        mutableBag.add(<(literal.(type))("4")>);
+        Assert.assertEquals(mutable4, mutableBag);
+        Assert.assertEquals(immutable3, immutableBag);
+    }
 }
 
 >>
diff --git a/unit-tests/src/test/java/org/eclipse/collections/impl/stream/PrimitiveStreamsTest.java b/unit-tests/src/test/java/org/eclipse/collections/impl/stream/PrimitiveStreamsTest.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021 The Bank of New York Mellon.
+ * Copyright (c) 2022 The Bank of New York Mellon and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * and Eclipse Distribution License v. 1.0 which accompany this distribution.
@@ -51,6 +51,7 @@
 import org.eclipse.collections.impl.factory.primitive.LongSets;
 import org.eclipse.collections.impl.factory.primitive.LongStacks;
 import org.eclipse.collections.impl.list.primitive.IntInterval;
+import org.eclipse.collections.impl.list.primitive.LongInterval;
 import org.junit.Assert;
 import org.junit.Test;
 
@@ -107,6 +108,21 @@ public void toImmutableIntBag()
         Assert.assertEquals(IntBags.mutable.ofAll(IntStream.rangeClosed(1, 10)), bag);
     }
 
+    @Test
+    public void toEmptyImmutableIntBag()
+    {
+        ImmutableIntBag bag = PrimitiveStreams.iIntBag(IntStream.empty());
+        Assert.assertEquals(IntBags.immutable.empty(), bag);
+    }
+
+    @Test
+    public void toImmutableIntBagWithOneElement()
+    {
+        ImmutableIntBag bag = PrimitiveStreams.iIntBag(IntStream.rangeClosed(1, 1));
+        Assert.assertEquals(IntInterval.oneTo(1).toBag(), bag);
+        Assert.assertEquals(IntBags.mutable.ofAll(IntStream.rangeClosed(1, 1)), bag);
+    }
+
     @Test
     public void toIntStack()
     {
@@ -171,6 +187,21 @@ public void toImmutableLongBag()
         Assert.assertEquals(LongBags.mutable.ofAll(LongStream.rangeClosed(1, 10)), bag);
     }
 
+    @Test
+    public void toEmptyImmutableLongBag()
+    {
+        ImmutableLongBag bag = PrimitiveStreams.iLongBag(LongStream.empty());
+        Assert.assertEquals(LongBags.immutable.empty(), bag);
+    }
+
+    @Test
+    public void toImmutableLongBagWithOneElement()
+    {
+        ImmutableLongBag bag = PrimitiveStreams.iLongBag(LongStream.rangeClosed(1, 1));
+        Assert.assertEquals(LongInterval.oneTo(1).toBag(), bag);
+        Assert.assertEquals(LongBags.mutable.ofAll(LongStream.rangeClosed(1, 1)), bag);
+    }
+
     @Test
     public void toLongStack()
     {
@@ -235,6 +266,20 @@ public void toImmutableDoubleBag()
         Assert.assertEquals(DoubleBags.mutable.ofAll(DoubleStream.of(1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0, 10.0)), bag);
     }
 
+    @Test
+    public void toEmptyImmutableDoubleBag()
+    {
+        ImmutableDoubleBag bag = PrimitiveStreams.iDoubleBag(DoubleStream.empty());
+        Assert.assertEquals(DoubleBags.immutable.empty(), bag);
+    }
+
+    @Test
+    public void toImmutableDoubleBagWithOneElement()
+    {
+        ImmutableDoubleBag bag = PrimitiveStreams.iDoubleBag(DoubleStream.of(1.0));
+        Assert.assertEquals(DoubleBags.mutable.ofAll(DoubleStream.of(1.0)), bag);
+    }
+
     @Test
     public void toDoubleStack()
     {