Is It "Bad Form" To Use A Map Function That Sends An Email In A Java Stream?

user_967671 0
Viewed 80

I am trying to use Java Streams to make the sequential processing of a list of customers run in parallel. This is a short-term band-aid to a problem that we are solving as part of re-architecting our entire system.

What I am starting with is a List<Customer> Customers structure that contains the customer contact information and all the relevant transaction data. Conceptually, the code I am replacing looks like:

long emailsSent = 0;
List<Customer> customers = methodLoadingAllrelevantData();
for (Customer customer: customers) {
    boolean isEmailSent = sendEmail(customer);
    if (isEmailSent) {
        emailsSent++;
    }
}

The sendMail(customer) function:

  1. Determines if an email should be sent
  2. Formats the email
  3. Attempts to send the email
  4. Returns true if the email was sent successfully

Not great code, but I am just trying to get some more speed from the existing code, not trying to make it better. The method and all its calls are 100-percent thread-safe.

I put it in the following stream structure:

ForkJoinPool limitedParallelThreadPool = new ForkJoinPool(numberOfThreads);
emailsSent = limitedParallelThreadPool.submit( () ->
    customers.stream().parallel()
        .map(this::_emailCustomer)
        .filter(b -> b == true).count()
).get();

This does work as expected, returning the same data as the sequential version.

My questions are: because the purpose of my method is to generates an email, is it poor practice for me to use a map function? Is there a better answer? I am, in effect, mapping a Customer to a boolean, but part of this mapping requires the process to trigger an email.

I was originally trying to use a forEach()operator, but I could not figure out how to get the count without adding state information to the sendMail function, which interferes with it being thread-safe.

java-stream java-8 java
2 Answers

Returns true if the email was sent successfully

It wouldn't be the worst idea to take advantage of the fact that your _emailCustomer method returns a boolean, so you can use Stream#filter instead of a combination of both Stream#map and Stream#filter:

customers.parallelStream()
         .filter(this::_emailCustomer)
         .count()

To answer your question, though, it would depend on the use-case whether or not Stream#map is the correct intermediate operation to use. According to the documentation of Stream#map, the Function argument that the method accepts must be:

a non-interfering, stateless function to apply to each element

If your _emailCustomer method is either interfering or stateful, then I would refrain from calling it within Stream#map, especially in a parallel context.

user_72946470

Since you don't care in which order those emails are getting sent, I'd say you kind of OK in this example. It's just that you are relying on side-effects of the map intermediate operation and potentially that can bite you. For example this:

Stream.of(1,2,3,4)
      .map(x -> x + 1)
      .count();

will not execute the map at all (starting with java-9), since all you want is count and map will not change the final count. Your example is safe from that since you are filtering, thus the final count is not known, thus map has to be executed. As said, for a parallel environment there is no guarantee about the order in which map will be executed.

It's a pity though that your sendEmail returns something at all, all the email services I wrote were more like a event thing - fire and forget; but I can't tell your exact scenario needed.

Think about the fact that your map operation will block, until you get a response back and that might trigger this part of the documentation that you need to look at:

A ForkJoinPool is constructed with a given target parallelism level; by default, equal to the number of available processors. The pool attempts to maintain enough active (or available) threads by dynamically adding, suspending, or resuming internal worker threads, even if some tasks are stalled waiting to join others

user_10593720
Related