Java Code Design Using ExecutorService That Calling Mutliple Services

Question

I am attempting to run 15 services parallely and each service will send mail to different set of customers. Extraction criteria will differ from every services

Service1, Service2, Service3.... Service15. Each class extends NotificationService class.

NotificationService class have methods extractRecipients(), sendMail(), sendSMS(), logNotification().

All Service classes[1 to 15] have process() method that will call NotificationService methods and do their job.

is this correct way to design the java code?

And below code looks ugly, is there any smart way to handle. Please someone help me.

public void startService() {

try {
    ExecutorService service = Executors.newFixedThreadPool(3);

    for (;;) {
        service.submit(new Service1(conn) {
                    public Object call(){
                        try {
                            process(conn, param2); // Passing connection & obj
                        } catch (Exception e) {
                            throw e;
                        }
                        return null;
                    }
                });

        service.submit(new Service2(conn) {
                    public Object call(){
                        try {
                            process(conn, param2);
                        } catch (Exception e) {
                            throw e;
                        }
                        return null;
                    }
                });

        // like above i have 15 services. so its ugly.
    }

} catch (InterruptedException e) {
    e.printStackTrace();
}
}

public boolean process(Connection conn) throws Exception {
try {
// getRecipientsList(serviceID);

// sendMail(recipientsList);

// logNotificationDetails(notificationList);
} catch (Exception e) {
}
}

Answer 1

public void startService() {

try {
    List<Bean> list = getServicesNotificationList();

    ExecutorService service = Executors.newFixedThreadPool(list.size); here list size should be 15

    for (int i=0; i < list.size(); i++) {

        service.submit(new NotificationService(conn, list[i]));
    }

    service.shutdown();

} catch (InterruptedException e) {
    e.printStackTrace();
}
}

Answer 2

Some points for improvement (not complete):

I am assuming that Service1, Service2, ... classes are derived from NotificationService, overriding the process method.

anonymous interface implementation of Callable<Object>:

service.submit(new Service1(conn) {
                public Object call(){
                    try {
                        process(conn, param2); // Passing connection & obj
                    } catch (Exception e) {
                        throw e;
                    }
                    return null;
                }
            });

should be moved to NotificationService class. This code then reduces to:

service.submit(new Service1(conn)); // Will call Service1's "process" implementation
service.submit(new Service2(conn)); // Will call Service2's "process" implementation
...

Edit:

What I have in mind is this setup:

abstract class NotificationService implements Callable<Object>{
  // ...

  // I don't know what types conn and param2 are, so ...
  protected abstract void process( ConnType conn, Param2Type param2 );

  @Override
  public Object call(){
      try {
              process(conn, param2); // Passing connection & obj
      } catch (Exception e) {
              throw e;
      }
      return null;
  }
}

You then can override process in Service1, Service2 ... to handle the process in their specific way.

class Service1 extends NotificationService{
    @Override
    protected void process( ConnType conn, Param2Type param2 ){
        // do process according to Service1's needs.
    }
}

---

Endless loop:

for(;;){
   service.submit(...
}

will block the caller of startService in that loop while new Tasks are added to the Executor forever.

If each execution leads to an email to the customer, he will already be flooded.

---

Calling startService every minute:

will result in a new ExecutorService that will be endlessly flooded with tasks every minute!

This will not only spam your customers but should also degrade performance pretty fast.

---

For a starter, you could change like so:

public void startService() {

try {
    ExecutorService service = Executors.newFixedThreadPool(3);


        service.submit(new Service1(conn)); // assuming callable impl is moved

        service.submit(new Service2(conn));

        // like above i have 15 services. so its ugly.
    service.shutdown();

} catch (InterruptedException e) {
    e.printStackTrace();
}
}

---

An alternative would be to use one ScheduledExecutorService on which you schedule each ServiceN to be executed every minute.

Answer 3

Use invokeAll of executor service.