Android Google Play Services code for GPS

I'm using Google Play Services for Location in my Android app. This is my first app so I'm not expecting that I have utilized everything perfectly.

My app is to get GPS/Date and send to a server database. I was hoping to learn (and everything works so far) if I've used Google Play Services code properly.

I can only test via Android Studio Project emulator as well.

```
public class MainActivity extends Activity implements ConnectionCallbacks,
OnConnectionFailedListener {

private static final String TAG = MainActivity.class.getSimpleName();
private final static int PLAY_SERVICES_RESOLUTION_REQUEST = 1000;
private GoogleApiClient mGoogleApiClient;
private Location mLastLocation;
private String mFormattedDate;
List tasks;
private Button btnNewShowLocation;
View v;

@Override
protected void onCreate(Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
setContentView(R.layout.activity_main);
btnNewShowLocation = (Button) findViewById(R.id.btnNewShowLocation);
tasks = new ArrayList<>();

if (checkPlayServices()) {
buildGoogleApiClient();
}

btnNewShowLocation.setOnClickListener(new View.OnClickListener() {
@Override
public void onClick(View v) {
if (checkPlayServices()) {
requestData("http://10.0.2.2/index.php");
} else {
Toast.makeText(MainActivity.this, "Network isn't available", Toast.LENGTH_LONG).show();
}

}
});
}

private void displayLocation() {

mLastLocation = LocationServices.FusedLocationApi.getLastLocation(mGoogleApiClient);
Calendar c = Calendar.getInstance();
System.out.println("Current time => " + c.getTime());
SimpleDateFormat df = new SimpleDateFormat("yyyy-MM-dd HH:mm");
mFormattedDate = df.format(c.getTime());
if (mLastLocation != null) {
double latitude = mLastLocation.getLatitude();
double longitude = mLastLocation.getLongitude();
} else {
Toast.makeTe

I'm not usually an android review, but here goes:

private static final String TAG = [...]
private final static int PLAY_[...]

I find it helpful to keep my attributes in the same order. Pick one and stick to it. either the TAG becomes private final static or the PLAY_SERVICES_... becomes private static final.

I personally prefer the latter, that's also what most code I've encountered uses.

mGoogleApiClient, mLastLocation and mFormattedDate follow systems hungarian notation. This is considered a relic from the last milennium and you should avoid it. Instead use googleApiClient, lastLocation and formattedDate.

tasks (as well as v) is package-private. According to the Principle of Information Hiding you should make fields as restricted as possible.

From what I can see they both could be private.

While we're at tasks. I don't see any reassignment for that. You should make it final and instead of creating it anew onCreate you could .clear() it. The mGoogleApiClient is also only assigned in buildGoogleApiClient... Unfortunately that doesn't mean it can be made final.

requestData("http://10.0.2.2/index.php");

This is hard-coded. Hard coded connections are usually a bad thing. You should extract that into a configuration.

RequestPackage p = new RequestPackage();
p.setMethod("POST");
p.setUri(uri);
p.setParams("longitude",    String.valueOf(mLastLocation.getLongitude()));
p.setParams("latitude",     String.valueOf(mLastLocation.getLatitude()));
p.setParams("date",         String.valueOf(mFormattedDate));
p.setParams("boolean",      "true");

I don't like the way you formatted this, because keeping things aligned when the code changes (or the server-side changes) is needless maintenance effort. Also it creates a disconnect between the paramter names and their
values, which can make spotting bugs harder.

if (resultCode != ConnectionResult.SUCCESS) {
    if (GooglePlayServicesUtil.isUserRecoverableError(resultCode)) {
        GooglePlayServicesUtil.getErrorDialog(resultCode, this,
                PLAY_SERVICES_RESOLUTION_REQUEST).show();
    } else {
        Toast.makeText(getApplicationContext(),
                "This device is not supported.", Toast.LENGTH_LONG)
                .show();
        finish();
    }
    return false;
}
return true;

Can be simplified. This is showing signs of excess indentation, also known as "arrow code". Make it easier on yourself and use early returns to keep lines short and cohesive:

if (resultCode == ConnectionResult.SUCCESS) {
    return true;
}
[...]

mGoogleApiClient = new GoogleApiClient.Builder(this)
        .addConnectionCallbacks(this)
        .addOnConnectionFailedListener(this)
        .addApi(LocationServices.API).build();

Don't do that. Don't push your own reference outside of your control from inside your class. That shows you're having too many responsibilities.

Instead you should use an (anonymous) inner class to implement the callback behaviour.

This also removes the dependency to ConnectionCallbacks and OnConnectionFailedListener from your inheritance hierarchy (that you seem to want to use, judging by the littering of protected modifiers in your code.

Overall the code is written cleanly, but contains some minor and medium violations of best practice. Keep it up :)

StackExchange Code Review Q#97382, answer score: 3