Optimize withAll on MutableIntBagFactoryImpl and ImmutableIntBagFactoryImpl

[donraab]

The code currently in withAll on both mutable and immutable implementations calls toArray on IntStream which if the Stream is very large can be extremely expensive and won't take advantage of a Bag's ability to store duplicates as simply increments in a count.

Current mutable factory code:

public MutableIntBag withAll(IntStream items)
{
    return this.with(items.toArray());
}

Current immutable factory code:

public ImmutableIntBag withAll(IntStream items)
{
    return this.with(items.toArray());
}

It would be much better to simply iterate over the Stream and add the results to a HashBag, converting to an ImmutableBag in the immutable factory.

[TheJavaGuy]

I will do it. Very probably this method in the ImmutableIntBagFactoryImpl also must be refactored not to use toArray:

@Override
public ImmutableIntBag withAll(IntIterable items)
{
    if (items instanceof ImmutableIntBag)
    {
        return (ImmutableIntBag) items;
    }
    return this.with(items.toArray());
}

[donraab]

Hi @TheJavaGuy, thanks for volunteering! Yes, refactoring this method makes sense as well. Thank you for looking around the code! I've assigned the issue to you.

[TheJavaGuy]

For mutable Bags it’s easy fix. But for immutable ones I don’t see the way out without bigger changes. In the end the only way to create immutable bag (e.g. ImmutableIntHashBag) is either:

• through constructor private ImmutableIntHashBag(int[] newElements), or
• through overloaded withAll/ofAll which in the end of the chain must call constructor

To call the constructor array must be filled, and I’m back to square one :(

[donraab]

I think it would be fine for the Immutable factory to call the Mutable factory with the primitive Stream and then call toImmutable() at the end.