2.x Operators ReactiveType::AmbArray + ReactiveType::AmbIterable vs ReactiveType::Amb

[VictorAlbertos]

It seems that for the implementation of the operators AmbArray and AmbIterable different strategies have been adopted depending on the reactive type.

• Single uses SingleAmbArray and SingleAmbIterable
• Maybe uses MaybeAmbArray and MaybeAmbIterable
• Completable uses CompletableAmbArray and CompletableAmbIterable
• Observable uses ObservableAmb
• Flowable uses FlowableAmb

I propose to adopt the same approach for Single, Maybe and Completable, merging xAmbArray and xAmbIterable into one single class, xAmb, just like Observable and Flowable.

I think this will increase the test coverage for the internal operators.

Should I create a PR?

[akarnokd]

Sure.

[VictorAlbertos]

I'm following the FlowableAmb operator implementation to create the SingleAmb operator implementation, regarding the conversion of the Iterable to Array.

FlowableAmb

if (sources == null) {
    sources = new Publisher[8];
    for (Publisher<? extends T> p : sourcesIterable) {
        if (count == sources.length) {
            Publisher<? extends T>[] b = new Publisher[count + (count >> 2)];
            System.arraycopy(sources, 0, b, 0, count);
            sources = b;
        }
        sources[count++] = p;
    }
}

If the sourcesIterable has less elements than 8, sources array will contain null references. It seems to be fine for FlowableAmb, somehow it will remove or ignore the null references later.

SingleAmb (current adaptation of SingleAmbArray)

if (sources == null) {
    sources = new SingleSource[8];

    int count = 0;
    for (SingleSource<? extends T> element : sourcesIterable) {
        if (count == sources.length) {
            SingleSource<? extends T>[] b = new SingleSource[count + (count >> 2)];
            System.arraycopy(sources, 0, b, 0, count);
            sources = b;
        }
        sources[count++] = element;
    }
}

But that's not the case for SingleAmb operator. This class validates that none of the sources value is null. So, I will need to reduce the value of the initial size of sources to 1, or remove all the null references at some point later.

But, all this overhead could be avoided just converting internally from array to Iterable and work always with Iterable.

Arrays.asList(sources)

Is there any particular reason for not be doing that (performance?)? And, what approach should I take for SingleAmb?

Thanks.

[akarnokd]

somehow it will remove or ignore the null

Just look a further bit down to see that count is used for pre-creating the consumers and the subscribe loop only considers that many elements from the input array whose tail has nulls (but should have checked for null returned by the iterable and array).

The separate implementations were there to reduce overhead and branching during subscription time. Null checks should be enforced everywhere.

[VictorAlbertos]

Thanks @akarnokd for the explanation.

I'll change the for-each loop implementation for a classic loop using the count as part of the termination expression.