Simple application for finding local restaurants

I was asked to create a test APK file by a company HR when I applied for an Android developers job,

Specification:


The goal is to develop a simple application which finds the local
restaurants. The application will contain 2 screens:


A list of nearby restaurants within 1 mile radius A map that shows the
restaurants and their location The application should be able to work
in an offline mode showing the latest places found so far (even if
these are outside the 1 mile radius). In addition, the user must have
an option to refresh the data regardless of the screen he currently
sees.


In order to get restaurant information please use the Google Places
API.

I put in my best effort and created the app and when I emailed them the source code, I was given the following feedback:


The team felt you didn't have enough knowledge about Android in
general - you use some slow ways to send data inside the application,
don't know the design guidelines for Android and provided an
application that looks like an IOS clone. We are not asking people to
do beautiful applications but implementing things that Google says
never do on Android is certainly a negative point during review.

I think that the above comments are a bit rude and unreasonable. Could an experienced Android developer PLEASE review my code and let me know if these comments are true and where have I done wrong!

This has completely ruined my day, and I am feeling very depressed that my code and knowledge of Android has been rated that bad.

RestaurantsActivity.java

```
RestaurantsActivity extends FragmentActivity implements
RestaurantListFragment.Contract {
private final static String TAG = RestaurantsActivity.class.getSimpleName();

private static final String TAG_LEFT_FRAG = "info_headers";

private RestaurantListFragment left_frag = null;

private DatabaseHelper mDBHelper = null;
private Boolean isInternetPresent = false;
private ConnectionDetector connectionDetector;
Googl

-
GooglePlaces.search could return empty list instead of null. It would save you a few null checks in clients. (Effective Java, Second Edition, Item 43: Return empty arrays or collections, not nulls)

-
Be careful about Double.toString:

urlString.append("&location=");
urlString.append(Double.toString(latitude));
urlString.append(",");
urlString.append(Double.toString(longitude));

Double.toString(0.000974) returns 9.74E-4 which might not be what you want. (Google Maps might or might not handle it.)

RestaurantListFragment

-
list could be renamed to result to express its purpose.

-
list could have a smaller scope. (Effective Java, Second Edition, Item 45: Minimize the scope of local variables It has a good overview on the topic.)

-
You could use only a while loop and cursor.moveToNext() to iterate over the cursor.

-
Guard clauses makes the code flatten.

-
You could remove some duplication with a helper method:

private String getString(Cursor cursor, String attributeName) {
    return cursor.getString(cursor.getColumnIndex(attributeName));
}

Usage:

place.setId(getString(cursor, DatabaseHelper.UNIQUEID));
place.setRef(getString(cursor, DatabaseHelper.REFERENCE));
...

-
Here is another one:

private double getDoubleWithDefault(Cursor cursor, String attributeName, 
        double defaultValue) {
    if (cursor.isNull(cursor.getColumnIndex(attributeName))) {
        return defaultValue;
    }
    return cursor.getDouble(cursor.getColumnIndex(attributeName));
}

Usage:

place.setLatitude(getDoubleWithDefault(cursor, DatabaseHelper.LAT, 0));
place.setLongitude(getDoubleWithDefault(cursor, DatabaseHelper.LNG, 0));

Here is an improved version of fetchFromDB:

private List fetchFromDB() {
    Cursor cursor = null;
    try {
        DatabaseHelper dbHelper = new DatabaseHelper(getActivity());
        cursor = dbHelper.getReadableDatabase().rawQuery("SELECT * FROM  " 
            + DatabaseHelper.TABLE, null);
        if (cursor == null) {
            return Collections.emptyList();
        }
        List list = new ArrayList();
        while (cursor.moveToNext()) {
            Place place = new Place();
            place.setId(getString(cursor, DatabaseHelper.UNIQUEID));
            place.setRef(getString(cursor, DatabaseHelper.REFERENCE));
            place.setName(getString(cursor, DatabaseHelper.NAME));
            place.setFormattedAddress(getString(cursor, DatabaseHelper.ADDRESS));
            place.setPhone(cursor.getString(cursor.getColumnIndex(DatabaseHelper.PHONE)));
            place.setLatitude(getDoubleWithDefault(cursor, DatabaseHelper.LAT, 0));
            place.setLongitude(getDoubleWithDefault(cursor, DatabaseHelper.LNG, 0));
            list.add(place);
        }
        return list;
    } catch (Exception e) {
        Log.d(TAG, e.toString());
        return Collections.emptyList();
    } finally {
        if (cursor != null && !cursor.isClosed()) {
            cursor.close();
        }
    }
}

StackExchange Code Review Q#46086, answer score: 8