Checking for Internet connectivity on Android

Question

I wrote this code to check an internet connection and was wondering if this is the best way to do this check.

public static boolean isNetworkAvailable(Context context) {
    boolean status = false;
    try {
        ConnectivityManager cm = (ConnectivityManager) context
                .getSystemService(Context.CONNECTIVITY_SERVICE);
        NetworkInfo netInfo = cm.getNetworkInfo(0);

        if (netInfo != null
                && netInfo.getState() == NetworkInfo.State.CONNECTED) {
            status = true;
        } else {
            netInfo = cm.getNetworkInfo(1);
            if (netInfo != null
                    && netInfo.getState() == NetworkInfo.State.CONNECTED)
                status = true;
        }
    } catch (Exception e) {
        e.printStackTrace();
        return false;
    }
    return status;
}

Answer 1

I am not familiar enough with Java to know what inside your try block could throw an exception or what sort of exceptions you might expect, however, simply printing the stack trace surely can't really be all that helpful.

First of all, any end user isn't going to see this information. Second of all, no developer will see this testing outside of the development environment.

Again, I'm not that familiar with Java, so I don't know what options are available, but you need to do one of a few things.

1. Show the exception to the user (or some information about it anyway). This is probably completely unnecessary. The network connection is either available or not, right?
2. Rethrow the exception and let the caller handle it. It can be useful to know there was an exception and we should retry the network connection later.
3. Log the exception in a way that the developer can access even when it happens in production. Error logs should be uploaded somewhere.
4. Just do nothing, and don't waste execution time printing the stack trace. If you chose this option, you probably want to find a way to leave the print stack trace in for development environments, but for production builds, you shouldn't be executing this code.

Now.... our code also has some magic numbers in it. I realize this isn't entirely your fault. I've visited the official documentation. I see, that somehow, this method simply takes an int and the documentation doesn't seem to clarify what the legitimate values are very well... but they are all defined as constants:

ConnectivityManager.TYPE_BLUETOOTH
ConnectivityManager.TYPE_DUMMY
ConnectivityManager.TYPE_ETHERNET
ConnectivityManager.TYPE_MOBILE
ConnectivityManager.TYPE_MOBILE_DUN
ConnectivityManager.TYPE_MOBILE_HIPRI
ConnectivityManager.TYPE_MOBILE_MMS
ConnectivityManager.TYPE_MOBILE_SUPL
ConnectivityManager.TYPE_VPN
ConnectivityManager.TYPE_WIFI
ConnectivityManager.TYPE_WIMAX

These are the constants we should be passing in when checking for a connection. Using these constants makes things a bit more readable. But moreover, I might modify our method to be more flexible. You're only checking for WIFI or MOBILE (I don't know enough about Android to know if this is excluding anything important). But we can write our function in such a way that we can check for whatever types the user needs.

public static boolean isNetworkAvailable(Context context, int[] networkTypes) {
    try {
        ConnectivityManager cm = (ConnectivityManager)context.getSystemService(Context.CONNECTIVITY_SERVICE);
        for (int networkType : networkTypes) {
            NetworkInfo netInfo = cm.getNetworkInfo(networkType);
            if (netInfo != null && netInfo.getState() == NetworkInfo.State.CONNECTED) {
                return true;
            }
        }
    } catch (Exception e) {
        return false;
    }
    return false;
}

Now our method is duplicating less code and its flexible enough to check for what ever types of networks the user happens to specify. If we want a method that doesn't require passing in networks and still does what your original implementation does, we can just write a wrapper method that passes mobile/wifi in to this method.