About long polling
To clarify about long polling, from wikipedia:
With long polling, the client requests information from the server exactly as in normal polling, except it issues its HTTP/S requests (polls) at a much slower frequency. If the server does not have any information available for the client when the poll is received, instead of sending an empty response, the server holds the request open and waits for response information to become available. Once it does, the server immediately sends an HTTP/S response to the client, completing the open HTTP/S Request. In this way the usual response latency (the time between when the information first becomes available and the next client request) otherwise associated with polling clients is eliminated.
In particular, paraphrasing the key points:
- The client issues its requests at a slow frequency: this is not true in your implementation, you poll the server in sequence without an explicit delay in between requests. But maybe you're ok with that.
- When the information is not available at the time of the request, the server holds the request open, and responds when the information is ready: this is to implement on the server side, of course, and not visible in the posted code.
About your main questions
Is it the right approach for the long polling, or do you want to suggest a better approach? Also, a IntentService
starts when you turn on the phone to run that code, would be better to use a Service
?
An AsyncTask
calling itself repeatedly... sounds a bit strange.
Also, the doc says this:
AsyncTasks should ideally be used for short operations (a few seconds at the most.) If you need to keep threads running for long periods of time, it is highly recommended you use the various APIs provided by the java.util.concurrent pacakge such as Executor, ThreadPoolExecutor and FutureTask.
Although a single polling action is short,
your desired behavior is repeated polling for presumably a long time (you did not include a termination condition).
As such, an IntentService
sounds like a better approach:
In most cases an IntentService
is the preferred way to simple background operations.
Code review
Although following the recommendations above you might well reimplement using an IntentService
,
the original code has some interesting issues worth reviewing:
The AsyncTask
is defined to return a String
, but it's not used at all.
You can get rid of that, and then you won't need the EntityUtils.toString
call anymore
The code in your post doesn't catch exception,
even though some should be thrown,
for example IOException
during HTTP requests.
You should add some error handling for that.
Instead of hardcoding the URL inside the class,
it would be better to make it a parameter (initialize in the constructor).
Something like this:
private class LongPolling extends AsyncTask<String, Void, Void> {
private final URI uri;
private LongPolling(URI uri) {
this.uri = uri;
}
@Override
protected Void doInBackground(String... params) {
try {
HttpClient httpClient = new DefaultHttpClient();
HttpPost httpPost = new HttpPost(uri);
httpClient.execute(httpPost);
} catch (IOException e) {
// server down? maybe you should let the client know!
}
return null;
}
@Override
protected void onPostExecute(Void aVoid) {
new LongPolling(uri).execute();
}
}