HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Synchronous and asynchronous client methods

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
synchronousasynchronousmethodsclientand

Problem

I am working on a project in which I need to have synchronous and asynchronous methods of my Java client. Some customer will call synchronously and some customer will call asynchronously, depending on their requirement.

Here is my Java client which has synchronous and asynchronous methods:

public class TestingClient implements IClient {

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

    // for synchronous
    @Override
    public String executeSync(ClientKey keys) {

        String response = null;
        try {

            Future handle = executeAsync(keys);
            response = handle.get(keys.getTimeout(), TimeUnit.MILLISECONDS);
        } catch (TimeoutException e) {

        } catch (Exception e) {

        }

        return response;
    }

    // for asynchronous
    @Override
    public Future executeAsync(ClientKey keys) {

        Future future = null;

        try {
            ClientTask ClientTask = new ClientTask(keys, restTemplate);
            future = service.submit(ClientTask);
        } catch (Exception ex) {

        }

        return future;
    }
}


Here is my ClientTask class, which implements the Callable interface. I am passing around the dependency using DI pattern in the ClientTask class. In the call method, I am just making a URL basis on machineIPAddress and using the ClientKeys which is passed to ClientTask class and then hit the server using RestTemplate and get the response back.

```
class ClientTask implements Callable {

private ClientKey cKeys;
private RestTemplate restTemplate;

public ClientTask(ClientKey cKeys, RestTemplate restTemplate) {
this.restTemplate = restTemplate;
this.cKeys = cKeys;
}

@Override
public String call() throws Exception {

// .. some code here
String url = generateURL("machineIPAddress");
String response = restTempl

Solution

If I haven't missed anything then ClientKey should be thread safe. Once you have build the ClientKey object, the parameterMap is only ever read and never modified.

However it relies on the fact that no malicious or buggy client isn't trying to modify in the meantime. Therefor I would consider changing the ClientKey object to protect against changes. Several options come to mind

-
Change your ClientKey constructor so it creates a readonly map:

private ClientKey(Builder builder) {
    this.userId = builder.userId;
    this.clientId = builder.clientId;
    this.remoteFlag = builder.remoteFlag;
    this.testFlag = builder.testFlag;
    this.parameterMap = Collections.unmodifiableMap(builder.parameterMap);
    this.timeout = builder.timeout;
}


This will prevent anyone from changing the map obtained via getParameterMap.

-
Make your ClientKey class implement Iterable> which allows iteration over the internal map rather than exposing the map through a get function.

-
Return an Iterable> from getParameterMap (and probably call it getParameters instead).

I'd prefer 2 or 3 because it enforces the readonly-ness in a much stronger way.

Update

Number 3 could be simply implemented as

Iterable> getParameters() {
    return parameterMap.entrySet();
}


or if you want to be really paranoid (to protect against someone trying to cast the return value back to a Set in order to try adding elements):

Iterable> getParameters() {
    return Collections.unmodifiableSet(parameterMap.entrySet());
}


Usage would be to replace this:

final Map paramMap = cKeys.getParameterMap();
    Set> params = paramMap.entrySet();
    for (Entry e : params) {
        url.append("&" + e.getKey());
        url.append("=" + e.getValue());
    }


with:

for (Entry e : cKeys.getParameters()) {
        url.append("&" + e.getKey());
        url.append("=" + e.getValue());
    }


The slight downside to that is that a user cannot perform fast lookups anymore on it to get the value for a specific parameter. If you want to support that then you'd need to add a getParameter(string parameterName) method to ClientKey.

Code Snippets

private ClientKey(Builder builder) {
    this.userId = builder.userId;
    this.clientId = builder.clientId;
    this.remoteFlag = builder.remoteFlag;
    this.testFlag = builder.testFlag;
    this.parameterMap = Collections.unmodifiableMap(builder.parameterMap);
    this.timeout = builder.timeout;
}
Iterable<Entry<string, string>> getParameters() {
    return parameterMap.entrySet();
}
Iterable<Entry<string, string>> getParameters() {
    return Collections.unmodifiableSet(parameterMap.entrySet());
}
final Map<String, String> paramMap = cKeys.getParameterMap();
    Set<Entry<String, String>> params = paramMap.entrySet();
    for (Entry<String, String> e : params) {
        url.append("&" + e.getKey());
        url.append("=" + e.getValue());
    }
for (Entry<String, String> e : cKeys.getParameters()) {
        url.append("&" + e.getKey());
        url.append("=" + e.getValue());
    }

Context

StackExchange Code Review Q#40076, answer score: 3

Revisions (0)

No revisions yet.