9

I'm trying to generate Order instances using the Stream API. I have a factory function that creates the order, and a DoubleStream is used to initialize the amount of the order.

private DoubleStream doubleStream = new Random().doubles(50.0, 200.0);

private Order createOrder() {
    return new Order(doubleStream.findFirst().getAsDouble());
}

@Test
public void test() {

   Stream<Order> orderStream = Stream.generate(() -> {
       return createOrder();
   });

   orderStream.limit(10).forEach(System.out::println);

 ...

}

If I initialize the Order instance using a literal (1.0), this works fine. When I use the doubleStream to create a random amount, the exception is thrown.

Manuel Jordan
  • 15,253
  • 21
  • 95
  • 158
Ole
  • 41,793
  • 59
  • 191
  • 359

6 Answers6

15

The answer is in the javadoc of Stream (emphases mine):

A stream should be operated on (invoking an intermediate or terminal stream operation) only once. This rules out, for example, "forked" streams, where the same source feeds two or more pipelines, or multiple traversals of the same stream. A stream implementation may throw IllegalStateException if it detects that the stream is being reused.

And in your code, you do use the stream twice (once in createOrder() and the other usage when you .limit().forEach()

fge
  • 119,121
  • 33
  • 254
  • 329
4

As said in other answers, Streams are single-use items and you have to create a new Stream each time you need one.

But, after all, this isn’t complicated when you remove all your attempts to store intermediate results. Your entire code can be expressed as:

Random r=new Random(); // the only stateful thing to remember

// defining and executing the chain of operations:
r.doubles(50.0, 200.0).mapToObj(Order::new).limit(10).forEach(System.out::println);

or even simpler

r.doubles(10, 50.0, 200.0).mapToObj(Order::new).forEach(System.out::println);
Holger
  • 285,553
  • 42
  • 434
  • 765
  • Wow - That's very elegant - I love it! – Ole Jan 16 '15 at 20:37
  • When I try it - Eclipse says: "The constructed object of type Order is incompatible with the descriptor's return type double". Thoughts? – Ole Jan 16 '15 at 20:51
  • 2
    I was in a hurry yesterday, hence the oversight, `map` has to be `mapToObj` as it changes the type from primitive type `double` to a reference type, fixed it. – Holger Jan 17 '15 at 10:18
1

As fge states, you can't (shouldn't) consume a Stream more than once.

Any idea how to fix this?

From the Javadoc of Random#doubles(double, double)

A pseudorandom double value is generated as if it's the result of calling the following method with the origin and bound:

double nextDouble(double origin, double bound) {
    double r = nextDouble();
    r = r * (bound - origin) + origin;
    if (r >= bound) // correct for rounding
       r = Math.nextDown(bound);
    return r;
}

Implement such a method and use it to get a new double value each time you need one instead of trying to get it from a DoubleStream. Possibly use a DoubleSupplier.

private final Random random = new Random();
private DoubleSupplier supplier = () -> nextDouble(random, 50.0, 200.0);

private Order createOrder() {

    return new Order(supplier.getAsDouble());
}

private static double nextDouble(Random random, double origin, double bound) {
    double r = random.nextDouble();
    r = r * (bound - origin) + origin;
    if (r >= bound) // correct for rounding
        r = Math.nextDown(bound);
    return r;
}

If you're not going to reuse the nextDouble method, you can inline the values 50.0 and 200.0.

Community
  • 1
  • 1
Sotirios Delimanolis
  • 274,122
  • 60
  • 696
  • 724
0

Thank you - that was very helpful. I also came up with a different implementation that works well for now:

private DoubleStream doubleStream = new Random().doubles(50.0, 200.0);

private List<Order> createOrders(int numberOfOrders) {
List<Order> orders = new ArrayList<>();
doubleStream.limit(numberOfOrders).forEach((value) -> {
    Order order = new Order(value);
    orders.add(order);
});
return orders;
}

Thanks again!

Ole

Ole
  • 41,793
  • 59
  • 191
  • 359
0

Your method could be a one-liner like this instead. You need to use mapToObj, not map

private List<Order> createOrders(int numberOfOrders) {
     return doubleStream.limit(numberOfOrders).mapToObj(Order::new).collect(Collectors.toList());
}
user2336315
  • 15,697
  • 10
  • 46
  • 64
  • That's really sweet! Thanks! – Ole Jan 16 '15 at 22:24
  • Hm...When I run createOrders(10) I still get IllegalStateException: stream has already been operated on or closed. – Ole Jan 16 '15 at 22:50
  • Never mind - something was wrong in the test. Great solution!! – Ole Jan 16 '15 at 23:01
  • OK - I figured out what the problem was. If I call createOrders more than once in the same method I get the exception. Any thoughts on how to get around this? Thanks again. – Ole Jan 17 '15 at 00:35
  • OK - FIgured it out. A new random has to be created with each call. So: private List createOrders(int numberOfOrders) { return new Random().doubles(50, 200).limit(numberOfOrders).mapToObj(Order::new).collect(Collectors.toList()); } – Ole Jan 17 '15 at 00:39
0

You should use Supplier function interface for your initialization like this

Supplier<Stream<Double>> streamSupplier = () -> (new Random().doubles(50.0, 200.0).boxed());

And change your way to get double like this

streamSupplier.get().findFirst().get()

Then it works normally.

Found this way from the post Stream has already been operated upon or closed Exception

David Pham
  • 1,673
  • 19
  • 17