Synchronous and asynchronous methods in a client library

I am working on a library in which I need to make synchronous and asynchronous methods in my client library.

My library does this:

The customer will use our library and they will call it by passing user_id. We will then construct a URL by using that userId and make an HTTP client call to that URL. We will then get a JSON string back after hitting the URL. And after we get the response back as a JSON string, then we will send that JSON String back to our customer as it is.

Now I need to have synchronous and asynchronous methods. Some customer will call the executeSynchronous method to get the same feature and some customer will call our executeAsynchronous method and with the executeAsynchronous method, they will call future.get in there code itself.

Interface:

public interface Client {

    // for synchronous
    public String executeSynchronous(final String userId);

    // for asynchronous
    public Future executeAsynchronous(final String userId);
}

And then I have SmartClient which implements the Client interface:

public class SmartClient implements Client {

    private RestTemplate restTemplate = new RestTemplate();
    private ExecutorService service = Executors.newFixedThreadPool(5);

    // for synchronous call
    @Override
    public String executeSynchronous(String userId) {

        String response = null;

        try {
            Future handle = executeAsynchronous(userId);
            response = handle.get(500, TimeUnit.MILLISECONDS);
        } catch (TimeoutException e) {
            e.printStackTrace();
        }

        return response;
    }

    //for asynchronous call
    @Override
    public Future executeAsynchronous(String userId) {
        Future future = null;

        try {
            Task task = new Task(userId, restTemplate);
            future = executor.submit(task);
        } catch (Exception ex) {
            e.printStackTrace();
        }

        return future;
    }
}

This simple class wil

public String executeSynchronous(final String userId);

Drop this final as it helps nobody.

• How about the exception handling?

Exception handling is always a pain, especially with checked exceptions. Usually, the best way is the simplest one, i.e., do nothing. Declare the exception to be thrown and let it blow. Someone up the stack will take care of it (it may be you) in a place where this makes sense.

e.printStackTrace();

This is rather wrong. You should log the message, printing to stderr may be redirected to somewhere where nobody sees it. What's worse, your method returns null and the caller has no way to find out what went wrong. Even worse, the caller does not expect null and will get an NPE one day (usually, when it creates the biggest damage).

@Override
public String executeSynchronous(String userId) {

    String response = null;

    try {
        Future handle = executeAsynchronous(userId);
        response = handle.get(500, TimeUnit.MILLISECONDS);
    } catch (TimeoutException e) {
        e.printStackTrace();
    }

    return response;
}

You're actually using two threads (the current one and one in the executor) where just one is needed. It should go the other way round: Let executeSynchronous do the work and submit a Callable calling it when asynchronous execution is needed.

• Is this the correct and efficient way of doing this problem?

I guess, it's correct, but concerning efficiency it's no good to block two threads instead of one (and there's also the small overhead of executor, which is useless in the synchronous case).


... Can we generalize interface?

Maybe... I guess you want many such methods... and an interface containing both versions would be twice as big; no good idea. There's

com.google.common.util.concurrent.Futures.immediateFuture(@Nullable V value)

in Guava, which could help. Instead of two methods, use two implementations of a single interface (synchronous and asynchronous).

public interface Client {
    public Future executeAsynchronous(String userId);
}

The synchronous implementation does the real job normally, but returns an immediateFuture instead of the value itself.

The asynchronous implementation calls the synchronous one using the executor. In case of 10+ methods I'd consider using a dynamic proxy.

Alternatively, you could add an argument

enum Synchronicity {SYNCHRONOUS, ASYNCHRONOUS}

(don't use a boolean as its meaning is not obvious) to all the interface methods.

StackExchange Code Review Q#69687, answer score: 5