9
\$\begingroup\$

I am working on a project in which I am supposed to make synchronous and asynchronous behavior of my client.

The customer will call our client with a userId and we will construct a URL from that userId and make an HTTP call to that URL and we will get a JSON string back after hitting the URL. And after we get the response back as a JSON string, we will send that JSON string back to our customer.

In this case, I need to have synchronous and asynchronous methods. Some customers will call the executeSynchronous method to get the same feature and some customers will call our executeAsynchronous method to get the data back.

Interface:

public interface Client {

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

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

SmartClient which implements the Client interface:

public class SmartClient implements Client {

    ExecutorService executor = Executors.newFixedThreadPool(5);

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

        String response = null;
        Future<String> handle = getHandle(userId);

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

        }

        return response;
    }

    // This is for asynchronous call
    @Override
    public Future<String> executeAsynchronous(String userId) {

        return getHandle(userId);

    }

    private Future<String> getHandle(String userId) {
        Future<String> future = null;

        Task task = new Task(userId);
        future = executor.submit(task);

        return future;

    }
}

Simple class which will perform the actual task:

class Task implements Callable<String> {

    private final String userId;

    public Task(String userId) {
        this.userId = userId;   
    }

    public String call() throws Exception {

        String url = createURL(userId);

        // make a HTTP call to the URL
        RestTemplate restTemplate = new RestTemplate();
        String jsonResponse = restTemplate.getForObject(url, String.class);

        return jsonResponse;
    }

    // create a URL
    private String createURL(String userId) {
        String generateURL = somecode;

        return generateURL;
    }
}
  1. Is this the correct and efficient way of doing this problem?
  2. How about the exception handling?
  3. Do I need any other catch blocks at any places?
\$\endgroup\$
10
\$\begingroup\$

Late answer to this question.....

There are a few things which should be considered when implementing a solution like this, and best-practice comes in to play here.

One nit-pick ... before we go further, your interface the methods executeAsynchronous and executeSynchronous, whereas your implementation has executeAsync and execute. This is a carelessly posted question?

What you have, at the moment, works, but is it the best way? I don't think so.

You code has a few features that are important:

  • executeSynchronous() - waits until you have a result, returns the result.
  • executeAsynchronous() - returns a Future immediately which can be processed after other things are done, if needed.
  • it has a private getHandle() method which does the hard work of creating a Callable, and returning a future.

Now, I have tried to list why there are problems with this, but, it's easier to actually show you a different approach, and explain why it is better.... so, here's a different approach, which reverses the logic.

First thing, we are going to make the interface a generic interface... why does it always have to return a String value for each URL? Also, why is it a Client? It is not really a client, but it is a RemoteCall. This is not the full Client, but a part of what it does when it talks to the server.... so, let's rename the Client interface, and make it generic.... Also, while we are at it, you need to declare that you throw some form of exception... my preference is for a new checked exception type, but maybe you can borrow an existing exception. What you have is not best-practice. Here's a decent exception to throw:

class RemoteCallException extends Exception {

    private static final long serialVersionUID = 1L;

    public RemoteCallException(String message, Throwable cause) {
        super(message, cause);
    }

    public RemoteCallException(String message) {
        super(message);
    }

}

Then, with that exception, we can complete the interface:

public interface RemoteCall<T> {

    // for synchronous
    public T executeSynchronous(final String action) throws RemoteCallException;

    // for asynchronous
    public Future<T> executeAsynchronous(final String action);
}

Right, this interface is now a better representation of the work it is supposed to do.

Now, here's a key feature I want you to consider.... at the moment, you may only have one 'action' you need to execute on the server, the part of translating a userid in to a JSOM response. But, I expect there is, or will be, more. Having to build a new implementation for your Client interface for every type of operation is a real PITA, especially when it requires building a new Callable / Task instance.... so, what we do is we create a convenient abstract class, which will do all the synchronous/asynchronous work for us....

Note, it does not implement executeSynchronous(), and it is still fully generic. It uses an executor service that is passed in.

One of the really huge advantages of reversing the logic (there is no Callable involved in the synchronous call) is that the work is done on the calling thread. In your implementation, you have two threads fully occupied, the calling thread waits for work to be done, and a thread in the ExecutorService is actually doing the work. By making the executeSynchronous() call the way I suggest, only one thread is occupied, and the thread that is busy is the same thread that calls executeSynchronous()

public abstract class AbstractRemoteCall<T> implements RemoteCall<T> {

    private final ExecutorService executor;

    public AbstractRemoteCall(ExecutorService executor) {
        this.executor = executor;
    }

    // note, final so it cannot be overridden in a sub class.
    // note, action is final so it can be passed to the callable.
    public final Future<T> executeAsynchronous(final String action) {

        Callable<T> task = new Callable<T>() {

            @Override
            public T call() throws RemoteCallException {
                return executeSynchronous(action);
            }

        };

        return executor.submit(task);
    }
}

OK, so, the above class deals with any asynchronous method calls, and delegates the synchronous calls to the actual implementations.

Now we get to your actual implementation you want to do, the userid -> JSON transformation. It is now as easy as:

public class SmartClient extends AbstractRemoteCall<String> {

    public SmartClient() {
        super(Executors.newFixedThreadPool(5));
    }

    @Override
    public String executeSynchronous(String userId) throws RemoteCallException {

        String url = createURL(userId);

        // make a HTTP call to the URL
        RestTemplate restTemplate = new RestTemplate();

        return restTemplate.getForObject(url, String.class);

    }

    // create a URL
    private String createURL(String userId) {
        String generateURL = somecode;

        return generateURL;
    }
}

The advantages of all these changes are:

  • the abstract class is the only place you need to worry about threading
  • in a synchronous call, the work is done on the calling thread
  • there is a better mechanism for error handling with the new RemoteCallException.
  • you can now have many different implementations of the abstract class, each implementation does their own type of restful work, and you don't need to worry about the threading in any of them.
  • the actual working classes (like SmartClient) do one thing, and one thing only.

There are other advantages.... but, what you have now, though it works, is not nearly as maintainable, extensible, and reliable as what it could be.

EDIT: Some use-cases

How would the code I recommend be used? Two situations.... Synchronous

// set up some system, need the JSON data
SmartClient sc = new SmartClient();
String json = sc.executeSynchronous(userid);

Collecting data asynchronous:

// Set up a number of things, timing can be in parallel.
try {
    SmartClient sc = new SmartClient();
    Future<String> jsonfut = sc.executeAsynchronous(userid);
    // set up some other things.
    // perhaps some thing that gets other data....
    //   this is a different class just like the SmartClient that can be asynchronous
    DataCLient dc = new DataClient();
    Future<String> defautsfut = dc.executeAsynchronous("getdefaults");
    Future<String> messagesfut = dc.executeAsyncrhonous("messages");
    String json = jsonfut.get(500, TimeUnit.MILLISECONDS); 
    String defaults = defaultsfut.get(); // no timeout
    String messages = messagesfut.get(); // no timeout
} catch (TimeoutException te) {
    // handle appropriately
}
\$\endgroup\$
  • \$\begingroup\$ Thanks a lot rolfl.. Appreciated your help on this.. And now it really makes sense what is the benefit with your approach as compared to mine.. But in my use case, some customers would like to take advantage of executeAsynchronous method as well which means that some customers will call executeAsynchronous method directly from there application which will return the future and they can use it in the way they want to use and sometimes they will call executeSynchronous method as well. But in your example I guess customers can only call executeSynchronous method directly..? \$\endgroup\$ – david Feb 10 '14 at 4:50
  • \$\begingroup\$ ... Or might be I am missing something. Can you show me an example how customers will call executeSynchronous and executeAsynchronous method.. \$\endgroup\$ – david Feb 10 '14 at 4:51
  • \$\begingroup\$ @user2809564 I think you are missing that the executeAsynchronous calls the executeSynchronous as a Callable/Future \$\endgroup\$ – rolfl Feb 10 '14 at 4:58
  • \$\begingroup\$ Aah.. I just saw that but I don't see any future.get call in the synchronous method. And how do I timeout the synchronous call if it is taking time greater than we have specified? \$\endgroup\$ – david Feb 10 '14 at 5:00
  • \$\begingroup\$ And this is slightly new approach for me so I am trying to understand how would I make a call to synchronous and asynchronous method from the outside application? So if you can provide an example then it will be of great help to me.. \$\endgroup\$ – david Feb 10 '14 at 5:01
1
\$\begingroup\$

This looks good. A synchronous call is always an async call plus a wait for the result. To make that clear, execute() should probably call executeAsync() instead of getHandle().

\$\endgroup\$

protected by Jamal Nov 6 '15 at 20:17

Thank you for your interest in this question. Because it has attracted low-quality or spam answers that had to be removed, posting an answer now requires 10 reputation on this site (the association bonus does not count).

Would you like to answer one of these unanswered questions instead?

Not the answer you're looking for? Browse other questions tagged or ask your own question.