Jeroen Bouwens

Ramblings about software engineering, technology, business and everything else

How Not to Handle Asynchronous Calls

From time to time, most developers some across pieces of code that makes them go “Why on earth did you do it THAT way?!” at whoever wrote it. And it’s a dirty little secret that sometimes that thought is followed up with “Oh, it was me”. So for that reason I am not in the habit of harshly criticizing a developer I’ve never met, who wrote a piece of code the way they did for reasons I don’t know.

Still, sometimes people write stuff that makes me go “Why did you do it THAT way?!”, and last week was one of those times. I came across a piece of code that fired off two asynchronous calls at once, and then waited for both results before continuing. This was done in a Java application, and went roughly (simplified) as follows:

Convoluted way to wait for multiple asynchronous results
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
class FinalResult{
    public String source;
    public String dest;
}

FinalResult finalResult = new FinalResult("", "");

pubic void foo(String sourceData, String destData){

    peer.registerListener(this);
    peer.asyncRequest(sourceData, sourceRequestId);
    peer.asyncRequest(destData, destRequestId);

}

public void onResult(result, requestId){

    switch(requestId)
    case sourceRequestId:
        finalResult = new FinalResult(result, finalResult.dest);
        if(finalResult.dest != ""){
            doFinalCall(finalResult);
        }
    break;

    case destRequestId:
        finalResult = new FinalResult(finalResult.source, result);
        if(finalResult.source != ""){
            doFinalCall(finalResult);
        }
    break;
    }
}

I’ve glossed over some details here, but I hope the gist is clear. In order to ensure that both callbacks have occurred, a severely contrived structure is set up, completely foregoing any synchronization mechanisms Java has built in. Also, the whole system is completely locked into handling only an exact number of callbacks. Part of my job was to make it possible to have an arbitrary number of callbacks, which this setup is simply incapable of dealing with.

Even though my experience with Java spans all of 5 weeks, I did figure out Java has several ways to deal with synchronization issues, and I came up with the following, which does the exact same thing, except for an arbitrary number of asynchronous requests:

Improved way to wait for an arbitrary number of asynchronous results
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
private List<String> finalResult = new ArrayList<String>();
private int requestId = 0;
private int callbackCounter = 0;

pubic void foo(List<String> inputData){

    peer.registerListener(this);

    synchronized(lockObject)
    {
        for(String data : inputData){
            peer.asyncRequest(data, requestId++);
        }

        lockObject.wait();
    }
    doFinalCall(finalResult);
}

public void onResult(result, requestId){

    finalResult.add(result);

    if(callbackCounter++ == requestId){
        synchronized(lockObject){
            lockObject.notify();
        }
    }
}

This is a much nicer solution, if you ask me, and a lot more generic, readable and maintainable. There is still some room for improvement though. For one, I would prefer to not have to use asyncRequest at all, but have nice synchronous calls there instead. But alas, this is the way it is going to be. The asynchronous part resides in a piece of code we cannot touch. Such is the life of a contractor.

The point is, I cannot for the life of me understand why someone would end up with the original code. This was a textbook example of making your own life difficult by not using what your tools have to offer, and that is a great way to kill your productivity.

Know your tools, even if it is a programming language!

Comments