Description
In my project, I use this library to get some json response from server, then update UI & save json string to SharedPreferences. Cause this string is some kind big, if save it in UI thread, that will be a hurt for responsiveness. Everytime spawn a new thread to do so seems ugly also. so i just wonder why not we parse response some time earlier in the threadpool thread instead of in the onSucess/onFailure callback, thus we can avoid ugly code like this:
@Override
public final void onSuccess(final int statusCode, final Header[] headers, final byte[] responseBytes) {
if (statusCode != HttpStatus.SC_NO_CONTENT) {
Runnable parser = new Runnable() {
@Override
public void run() {
try {
final Object jsonResponse = parseResponse(responseBytes);
postRunnable(new Runnable() {
@Override
public void run() {
if (jsonResponse instanceof JSONObject) {
onSuccess(statusCode, headers, (JSONObject) jsonResponse);
} else if (jsonResponse instanceof JSONArray) {
onSuccess(statusCode, headers, (JSONArray) jsonResponse);
} else if (jsonResponse instanceof String) {
onFailure(statusCode, headers, (String) jsonResponse, new JSONException("Response cannot be parsed as JSON data"));
} else {
onFailure(statusCode, headers, new JSONException("Unexpected response type " + jsonResponse.getClass().getName()), (JSONObject) null);
}
}
});
} catch (final JSONException ex) {
postRunnable(new Runnable() {
@Override
public void run() {
onFailure(statusCode, headers, ex, (JSONObject) null);
}
});
}
}
};
if (!getUseSynchronousMode()) {
new Thread(parser).start();
} else {
// In synchronous mode everything should be run on one thread
parser.run();
}
} else {
onSuccess(statusCode, headers, new JSONObject());
}
}my basic thoughts are:
- use org.apache.http.client.ResponseHandler to specify what type we want parse the response to; make our ResponseHandlerInterface extends from it;
- some related changes:
abstract class AsyncGenericResponseHandler<T> implements ResponseHandlerInterface<T>
public abstract class AsyncHttpResponseHandler extends AsyncGenericResponseHandler<byte[]>
public abstract class TextHttpResponseHandler extends AsyncGenericResponseHandler<String>- AsyncGenericResponseHandler is basically the same as original AsyncHttpResponseHandler, with 2 important methods like:
@Override
public void sendResponseMessage(HttpResponse response) throws IOException {
// do not process if request has been cancelled
if (!Thread.currentThread().isInterrupted()) {
StatusLine status = response.getStatusLine();
T responseBody = handleResponse(response);
// additional cancellation check as getResponseData() can take non-zero time to process
if (!Thread.currentThread().isInterrupted()) {
if (status.getStatusCode() >= 300) {
sendFailureMessage(status.getStatusCode(), response.getAllHeaders(), responseBody,
new HttpResponseException(status.getStatusCode(), status.getReasonPhrase()));
} else {
sendSuccessMessage(status.getStatusCode(), response.getAllHeaders(), responseBody);
onParseResponseDone(responseBody);
}
}
}
}
@Override
public void onParseResponseDone(T responseBody) {
// default do nothing...
}then all subclass just need to override the handleResponse(HttpResponse) method to return the proper type. You may want to see the rough changes on t/tmp in my repo.
Didn't know the original intention, but for me, parse the response in the same background thread makes sense, also other operations on response(like save it to DS). IMO, we should make full use of background thread as it didn't hurt performance.