3
\$\begingroup\$

I have written this class to check internet connectivity on Android, while there are many instances when WiFi is connected is taken as an implication that internet is connected as well. I wanted to have a method which actually checks if internet is connected. One issue was to run the internet code in a background thread and while doing so - return a value, so I used a future task, below is the code:

public class CheckWiFi {
public static String TAG = "Check Internet";


public static boolean isWiFiConnected(Context ctx) {
    ConnectivityManager cm = (ConnectivityManager) ctx.getSystemService(Context.CONNECTIVITY_SERVICE);
    NetworkInfo ni = cm.getActiveNetworkInfo();
    return ni != null;
}


public static Boolean isNetworkConnected(Context ctx) {

    ExecutorService service = Executors.newFixedThreadPool(10);

    FutureTask<Boolean> task1 = new FutureTask<Boolean>(new CheckConnection(ctx));
    service.submit(task1);
    try {
        return task1.get();
    } catch (java.util.concurrent.ExecutionException ex) {
        ex.printStackTrace();
        return false;
    } catch (InterruptedException ex) {
        ex.printStackTrace();
        return false;
    }


}


public static class CheckConnection implements Callable {
    Context ctx;

    public CheckConnection(Context ctx) {
        this.ctx = ctx;
    }

    @Override
    public Boolean call() {
        if (isWiFiConnected(ctx)) {
            try {
                HttpURLConnection urlc = (HttpURLConnection)
                        (new URL("http://clients3.google.com/generate_204").openConnection());
                urlc.setRequestProperty("User-Agent", "Android");
                urlc.setRequestProperty("Connection", "close");
                urlc.setConnectTimeout(1500);
                urlc.connect();
                return (urlc.getResponseCode() == 204 && urlc.getContentLength() == 0);
            } catch (IOException e) {
                Log.e(TAG, "Error checking internet connection", e);
            }
        } else {
            Log.d(TAG, "No network available!");
        }
        return false;
    }

}

}

What are your thoughts about it?

Also that I am getting a warning:

Unchecked assignment: 'network.CheckWiFi.CheckConnection' to 'java.util.concurrent.Callable<java.lang.Boolean>'
\$\endgroup\$
  • 1
    \$\begingroup\$ As to the warning, change CheckConnection implements Callable to CheckConnection implements Callable<Boolean>. \$\endgroup\$ – JimmyB Feb 12 '16 at 9:37
  • \$\begingroup\$ Are you trying to implement a BroadcastReceiver here? More specifically a NetworkReceiver? \$\endgroup\$ – Bruno Costa Feb 12 '16 at 10:33
  • \$\begingroup\$ No while a broadcast receiver for sniffing network state is a good idea, however the use case here is to check the data connection at the start of the application. So that data download can be initiated. This method can be used as a start for a network sniffer making the network state to true at the start. \$\endgroup\$ – Skynet Feb 12 '16 at 11:04
1
\$\begingroup\$

Your implementation of isNetworkConnected does not make much sense for the following reasons:

  1. You create a new, private ExecutorService each time the method runs. The intention behind Executors is that they're created once and then used over a longer time span to execute multiple (short) tasks. This should save you from having to create a new Thread for each task, because the executor manages a number of threads which get re-used for multiple tasks in succession.

  2. You don't need that FutureTask<Boolean>. The usual idiom of the ExecutorService is like:

    Callable myCallable = new CheckConnection(...); Future future = service.submit( myCallable ); ... Boolean result = future.get();

  3. It does not make any sense to submit a (single) task to an executor and then start waiting for completion immediately. This will block exactly as if there was no Executor involved at all. In your case, you could just write Boolean result = new CheckConnection(...).call(); with the same effect w.r.t. concurrency.

Suggestion

Don't run your checking code and wait for the result. Better have your checking code run in the background periodically (e.g. in a dedicated Thread or a ScheduledExecutorService) and have that code post its result back to any interested party.

You can use a Handler to send a message from a background thread to the UI thread. For example, send one message each time the connectivity state changes.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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