eclipse / eclipse-collections

Eclipse Collections is a collections framework for Java with optimized data structures and a rich, functional and fluent API.
https://eclipse.dev/collections/
2.42k stars 606 forks source link

Implement a better way of performing non-short-circuit boolean operations. #720

Closed motlin closed 4 years ago

motlin commented 5 years ago

I'm working with an API that performs a mutation and then returns a boolean to indicate if a mutation happened, similar to Map.put(). I want to iterate over a collection, possibly perform mutations for each item, and then answer whether any mutations happened. I'm finding this awkward with the available API in Eclipse Collections 10 and can think of a few additions that might improve things.

  1. Non-short-circuit alternatives

For example RichIterable.anySatisfyNonShortCircuit(predicate)

  1. Helper methods on BooleanIterable

My current workaround is something like:

ImmutableBooleanList immutableBooleanList = iterable
        .collectBoolean(each -> this.possiblyPerformMutation(each, extraContext1, extraContext2));
boolean mutationOccurred = immutableBooleanList.anySatisfy(x -> x);

I don't mind the call to collectBoolean() but the call to anySatisfy() with an identity function feels unnecessary. It feels like BooleanIterable should have helper methods like any() and all().

default boolean any()
{
    return this.anySatisfy(x -> x);
}

default boolean all()
{
    return this.allSatisfy(x -> x);
}

default boolean none()
{
    return this.noneSatisfy(x -> x);
}
motlin commented 5 years ago
  1. A non-short-circuit decorator

Option 1 would add a lot of new API. A better alternative might be implementing non-short-circuit decorators. RichIterable.asNonShortCircuit() could return a decorator where all the short circuit methods are overridden to iterate through the full backing collection.

mohrezaei commented 5 years ago

Obligatory reminder: this is trivial in a for loop:

boolean mutated = false;
for(x ...)
{
    mutated |= possiblyMutate(x);
}
mohrezaei commented 5 years ago

Sheesh... can't get a comment out of the functional peanut gallery with a for loop jab :stuck_out_tongue: ? I guess I'll have to put my itchy functional hat on:

    @Test
    public void boolLongCircuit()
    {
        FastList<AtomicInteger> list = FastList.newList();
        for (int i=0;i<100;i++)
        {
            list.add(new AtomicInteger(i));
        }
        RichIterable<AtomicInteger> it = list;
        boolean changed = it.injectInto(0, (int i, AtomicInteger x) -> 
            incrementIfEven(x) ? i + 1 : i
        ) > 0;
        Assert.assertTrue(changed);
    }

    private boolean incrementIfEven(AtomicInteger i)
    {
        if ((i.get() & 1) == 0)
        {
            i.incrementAndGet();
            return true;
        }
        return false;
    }

This is just a fold operation: I used the int version cause we don't have a boolean one. Which is what I dislike about functional code: it always feels like solving a puzzle.

donraab commented 5 years ago

@motlin Sorry to be late to the discussion here, but couldn't this behavior be accomplished using count? Count is non-short circuiting and takes a predicate.

int count = iterable
        .count(each -> this.possiblyPerformMutation(each, extraContext1, extraContext2));
boolean mutationOccurred = count > 0;
mohrezaei commented 5 years ago

@donraab agreed. I'd forgotten about count. There is even a countWith

motlin commented 5 years ago

I like that. I’ll name the variable numberOfMutations and the code will be pretty clear. Thanks!

On Mon, May 20, 2019 at 9:06 PM Mohammad Rezaei notifications@github.com wrote:

@donraab https://github.com/donraab agreed. I'd forgotten about count. There is even a countWith

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/eclipse/eclipse-collections/issues/720?email_source=notifications&email_token=AAB3UIRVXY5KPZRZ4HQ2X4TPWNDHZA5CNFSM4HK4IDH2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODV2OV4A#issuecomment-494201584, or mute the thread https://github.com/notifications/unsubscribe-auth/AAB3UIREFD44CSVWSH3TZW3PWNDHZANCNFSM4HK4IDHQ .

vmzakharov commented 4 years ago

Given the consensus between @donraab @mohrezaei @motlin can this issue be marked as closed?