Effective use of multiple streams

I am experimenting with streams and lambdas in Java 8. This is my first serious foray using functional programming concepts; I'd like a critique on this code.

This code finds the Cartesian product of three sets of ints. My questions/complaints about this snippet are:

• Are nested flatMaps really the best (most understandable, most CPU/memory efficient) way to do an arbitrary Cartesian product? Or should I stick to imperative style for Cartesian products?

• Is there a better way to format nested lambdas (see, e.g., my use of flatMap)?

I just learned about Collectors; those might be what I need. I'm still learning how to best combine these things.

long count = IntStream.rangeClosed(0,9) /* 0  
            IntStream.rangeClosed(0,9) /* 0 
                    IntStream.rangeClosed(0,9) /* 0  (new Product()).A(a).B(b).C(c))
            )
     ).count();
System.out.println("Enjoy your Cartesian product." + count);

UPDATE: I deliberately omitted boilerplate code and class/method definitions, for brevity. The only important thing about Product is that it stores three ints.

Your code is odd in the sense that it is going to a lot of effort to calculate that 103 is 1000.

I understand why you are doing it, but I took the liberty of changing the count() terminating function and replacing it with:

StringBuilder sb = new StringBuilder();

.......
.forEachOrdered(prod -> sb.append(prod.toString()).append("\n");

This way we can store each result, instead of just counting it.

There are a few things to go through here.

Product

This class looks awkward:

.mapToObj(c -> (new Product()).A(a).B(b).C(c))

Why not just have a useful constructor on Product like:

public Product(int a, int b, int c) {
        super();
        this.a = a;
        this.b = b;
        this.c = c;
    }

Then, your mapping would look like:

.mapToObj(c -> new Product(a, b, c))

Magic Numbers

You have 0 and 9 as magic numbers in multiple places. These should be declared as (effective) constants, or calculated somehow. With the 9 value especially, if you want to change the size of the product you have to change it in three places.

int vs. Integer

You start off with an IntStream, but then convert it to a Stream. Why? If you can leave things as primitives, you should.

The actual stream

The most concerning aspect is the stream itself.

The flatMap operation is not doing what I think you think it does....

Let's consider the contents of the stream as it goes through things:

• 'a' range from 0-9 IntStream

• convert each int to an Integer

• for each Integer, flatMap that Integer

• the flatMap creates an IntStream which it converts to a Stream

• even though we are flatMapping the 'a' value, we are not using the 'a' value in the mapping. All we are really doing is creating a new 'b' value, and who cares that the stream now has the b values streaming through....

• with these 'b' values on the stream, we then flatMap again....

• again, we ignore what the actual 'b' value is, abut we generate a new 'c' value in a third nested stream, and then we combine the 'a' and 'b' values which are 'in scope', with the 'c' value from the stream, and we make a Product.

The point of what I am saying is that you actually are only using the Stream in the most inside IntStream, the 'c' stream. All the other streams are just 'tokens' that you need to count the events you are doing. A for-loop is the right thing for this construct (if it was not for the parallelism). If you include the parallelism, then a fork-join process is right. Not a stream.

My Attempt

I am not claiming this is right, or best practice, but consider this attempt to get essentially the same result as you:

/** Append an int to an array after the last array item **/
public static final int[] append(int[] base, int val) {
    int[] ret = Arrays.copyOf(base, base.length + 1);
    ret[base.length] = val;
    return ret;
}

/** Stream an int range and map to an appended array */
private static final Stream appendInts(int[] base, int size) {
    return IntStream.rangeClosed(0, size).mapToObj(a -> Cartesian.append(base, a));
}

public static void main(String[] args) {

    final int last = 9;
    final StringBuilder sb = new StringBuilder();
    IntStream.rangeClosed(0,last) /* 0  new int[]{a})
             // stream is int[1] array
             .flatMap(ab  -> appendInts(ab, last))
             // stream is int[2] array
             .flatMap(abc -> appendInts(abc, last))
             // stream is int[3] array
             .forEachOrdered(res -> sb.append(Arrays.toString(res)).append("\n"));;
    System.out.println("Enjoy your Cartesian product." + sb.toString());
}

Update

I have rewritten the OP's code to extract the stream preparations. Consider the following code:

// convert two input values to a stream consisting of an arrays of int
    BiFunction> stage3 = (a,b) -> {
        return IntStream.rangeClosed(0, last).mapToObj(c -> new int[]{a, b, c});
    };

    // convert an input value to a stream consisting of an arrays of int
    Function> stage2 = a -> {
        Stream s = IntStream.rangeClosed(0, last).boxed();
        Stream ret = s.flatMap(b -> stage3.apply(a, b));
        return ret;
    };

    IntStream.rangeClosed(0,last) /* 0  stage2.apply(a))
            .map(r -> Arrays.toString(r))
            .forEachOrdered(System.out::println);

Which is essentially what the OP's code does.

StackExchange Code Review Q#45026, answer score: 15