3

As a refactoring exercise, I'm trying to take something like this:

    for (int i = 1; i < orderedList.size(); i++) {
        Object object1 = ordered.get(i - 1);
        Object object2 = ordered.get(i);
        if (isDifferent(object1, object2)) return true;
    }

    return false;

into a simple Java functional statement. At first glance, I'm processing elements of a list sequentially, so reduce() sounded promising, but

  1. It would require carrying along a boolean value, which loses me the previous element; or
  2. carrying along the previous element of the list, which means I need to put it in some kind of wrapper object, or kludgy use of Optional or another List as a wrapper, in order to keep track of whether I have a difference or not.

So, I see ways I can use a reduce operation, but it would be more complicated and hard to understand than the for-loop original.

I looked at creating a custom collector, but ran into the same issues.

Is this a proper use of reduce, or am I just tempted because I know I can use it to process sequential values against a previous iteration? If it's a proper use, how can I compose the functions to reduce myself to a boolean?

Thanks for help with this thought exercise.

orbfish
  • 7,381
  • 14
  • 58
  • 75
  • 1
    Streams suit processing each element in isolation, or adjacent elements when reducing. You can “abuse” the pattern by reading/writing to variables outside the stream, but at that point you’re better off with an old school loop. – Bohemian Aug 03 '19 at 23:56
  • 1
    What's wrong about just leaving it? The stream will deprive you of the possibility to short-circuit. So performance-wise you'll be worse off with streams, unless `isDifferent` evaluates to `true` exceedingly rarely. In terms of readability you've already seen yourself that it doesn't play out nicely. –  Aug 03 '19 at 23:59
  • @Bohemian Reduction is not defined as an operation processing adjacent elements; it requires an associative function, so it may process, e.g. `(a×b)×(c×d)`, which does not fit the OP’s task. You’d have to map all elements to a type holding an interval (first and last element) and the boolean state. Besides the complexity of that, you’d lose the short-circuiting… – Holger Aug 13 '19 at 14:55

3 Answers3

2

Because you're indexing two Lists with a for-loop, you can replace it with an IntStream and reduce it with IntStream#anyMatch:

return IntStream.range(1, orderedList.size())
                .anyMatch(i -> isDifferent(ordered.get(i - 1), ordered.get(i)));

Although, I don't really see this providing much benefit, so it may be more readable to just keep it as a for-loop.

Jacob G.
  • 28,856
  • 5
  • 62
  • 116
  • Actually, I kind of like that. I was trying to get rid of the "i-1" with the reduce operation, and when i shoved i+1/i-1 in a `forEach` it looked worse than the original, but this is more readable even with the manual indexing. – orbfish Aug 04 '19 at 23:23
1

The fundamental operation here is called "zipping": given two streams of As and Bs and a combining operator (A, B) -> C, you can create a stream of Cs (truncating to the shorter input stream). Assuming you have such a function

<A, B, C> Stream<C> zip
 (Stream<? extends A> as, Stream<? extends B> bs,
  BiFunction<? super A, ? super B, ? extends C> combine);

You can implement your operation as

zip(ordered.stream(), ordered.stream().skip(1), this::isDifferent).anyMatch(x -> x);
// ordered: a b c d ... y z
// skipped: b c d ... y z
// zipped : (a, b) (b, c) (c, d) ... (y, z)

There is no zip in the standard library. If you're using something like Guava, you may check whether it has this operation, e.g. here's one. Or, you can implement it yourself and stick it in some utility class, at which point you may want to check out this answer. I will reproduce the code listed there (which appears to be cribbed from a beta version of the actual Streams API):

public static<A, B, C> Stream<C> zip(Stream<? extends A> a,
                                     Stream<? extends B> b,
                                     BiFunction<? super A, ? super B, ? extends C> zipper) {
    Objects.requireNonNull(zipper);
    Spliterator<? extends A> aSpliterator = Objects.requireNonNull(a).spliterator();
    Spliterator<? extends B> bSpliterator = Objects.requireNonNull(b).spliterator();

    // Zipping looses DISTINCT and SORTED characteristics
    int characteristics = aSpliterator.characteristics() & bSpliterator.characteristics() &
            ~(Spliterator.DISTINCT | Spliterator.SORTED);

    long zipSize = ((characteristics & Spliterator.SIZED) != 0)
            ? Math.min(aSpliterator.getExactSizeIfKnown(), bSpliterator.getExactSizeIfKnown())
            : -1;

    Iterator<A> aIterator = Spliterators.iterator(aSpliterator);
    Iterator<B> bIterator = Spliterators.iterator(bSpliterator);
    Iterator<C> cIterator = new Iterator<C>() {
        @Override
        public boolean hasNext() {
            return aIterator.hasNext() && bIterator.hasNext();
        }

        @Override
        public C next() {
            return zipper.apply(aIterator.next(), bIterator.next());
        }
    };

    Spliterator<C> split = Spliterators.spliterator(cIterator, zipSize, characteristics);
    return (a.isParallel() || b.isParallel())
           ? StreamSupport.stream(split, true)
           : StreamSupport.stream(split, false);
}
HTNW
  • 27,182
  • 1
  • 32
  • 60
  • Hadn't thought of zipping the same list into itself, and didn't realize any (relatively) standard libraries supported zip. Thank you! – orbfish Aug 04 '19 at 23:37
1

Isn't it is as simple as this from your logic?

return orderedList.stream.distinct().count() != 1; // assuming that you have equals and 
                                                  //  hashcode method overridden for 
                                                  // your object.
umamahesh
  • 41
  • 4