patternjavaMinor
App to list books from JSON data
Viewed 0 times
booksappjsonlistfromdata
Problem
I recently submitted the code for this for an 2 hour interview but I was rejected.
The app shows book information with a image, title, and author. Each entry has a title but some entries have no image and/or author.
Here is a screenshot:
I was given this feedback:
There is a lack of proficiency from what we would expect, specifically the data side. We saw inconsistencies with structure, style, and spacing, as well as, some deprecated code (Though addressed in your review).
Of course, no response was given when questioned, so I'm here. Please review my code. If possible, please tell me how to improve it. To be honest, I don't think it's that bad as to reject me as the app fully works, but I don't really know what they were looking for.
Here is my relevant code:
MainActivity
```
import android.content.Context;
import android.net.ConnectivityManager;
import android.net.NetworkInfo;
import android.os.AsyncTask;
import android.os.Bundle;
import android.support.v7.app.AppCompatActivity;
import android.support.v7.widget.Toolbar;
import android.view.Menu;
import android.view.MenuItem;
import android.widget.ListView;
import org.apache.http.HttpEntity;
import org.apache.http.HttpResponse;
import org.apache.http.client.HttpClient;
import org.apache.http.client.methods.HttpGet;
import org.apache.http.impl.client.DefaultHttpClient;
import org.apache.http.util.EntityUtils;
import org.json.JSONArray;
import org.json.JSONException;
import org.json.JSONObject;
import org.json.JSONTokener;
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
public class MainActivity extends AppCompatActivity {
private ListView mListView;
@Override
protected void onCreate(Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
setContentView(R.layout.activity_main);
Toolbar toolbar = (Toolbar) findViewById(R.id.toolbar);
setSupportActionBar(toolbar);
mListView = (ListView) findViewById(R.id.book_l
The app shows book information with a image, title, and author. Each entry has a title but some entries have no image and/or author.
Here is a screenshot:
I was given this feedback:
There is a lack of proficiency from what we would expect, specifically the data side. We saw inconsistencies with structure, style, and spacing, as well as, some deprecated code (Though addressed in your review).
Of course, no response was given when questioned, so I'm here. Please review my code. If possible, please tell me how to improve it. To be honest, I don't think it's that bad as to reject me as the app fully works, but I don't really know what they were looking for.
Here is my relevant code:
MainActivity
```
import android.content.Context;
import android.net.ConnectivityManager;
import android.net.NetworkInfo;
import android.os.AsyncTask;
import android.os.Bundle;
import android.support.v7.app.AppCompatActivity;
import android.support.v7.widget.Toolbar;
import android.view.Menu;
import android.view.MenuItem;
import android.widget.ListView;
import org.apache.http.HttpEntity;
import org.apache.http.HttpResponse;
import org.apache.http.client.HttpClient;
import org.apache.http.client.methods.HttpGet;
import org.apache.http.impl.client.DefaultHttpClient;
import org.apache.http.util.EntityUtils;
import org.json.JSONArray;
import org.json.JSONException;
import org.json.JSONObject;
import org.json.JSONTokener;
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
public class MainActivity extends AppCompatActivity {
private ListView mListView;
@Override
protected void onCreate(Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
setContentView(R.layout.activity_main);
Toolbar toolbar = (Toolbar) findViewById(R.id.toolbar);
setSupportActionBar(toolbar);
mListView = (ListView) findViewById(R.id.book_l
Solution
I see two main problems:
1) Formatting problems:
-
Inconsistent formatting in the braces, which would drive me crazy if I were interviewing a candidate. It's sometimes in the same line and sometimes in the next.
-
Unnecessary newlines here and there, while missing others (between closing a function and starting the next one).
This is either because you took code from somewhere else and did not adapt it, or your formatting is inconsistent. It can be fixed in almost any code editor with a simple command, and there is no excuse for not using it for code submitted for an interview.
2) You handle improperly errors:
-
You left an important part unfinished (
-
If for any reason the books cannot be retrieved, the code does nothing. You even left the TODO for showing a Toast. If you know you can show a Toast, why didn't you do it? This feels like laziness.
-
You are using
And other stuff:
-
I guess the deprecated code refers to
-
Leaving URLs in the middle of the code (
1) Formatting problems:
-
Inconsistent formatting in the braces, which would drive me crazy if I were interviewing a candidate. It's sometimes in the same line and sometimes in the next.
-
Unnecessary newlines here and there, while missing others (between closing a function and starting the next one).
This is either because you took code from somewhere else and did not adapt it, or your formatting is inconsistent. It can be fixed in almost any code editor with a simple command, and there is no excuse for not using it for code submitted for an interview.
2) You handle improperly errors:
-
You left an important part unfinished (
//can't do networking on device). This is important because in real-world apps, error managing of this type is what takes most of the time and effort.-
If for any reason the books cannot be retrieved, the code does nothing. You even left the TODO for showing a Toast. If you know you can show a Toast, why didn't you do it? This feels like laziness.
-
You are using
printStackTrace(); instead of logging. That's almost useless in Android.And other stuff:
-
I guess the deprecated code refers to
HttpClient. It's been marked as deprecated for a while, and it was removed from Android 6. This means that, right now, this code is not production ready.-
Leaving URLs in the middle of the code (
"http://de-coding-test.s3.amazonaws.com/books.json") is not nice, and it takes just some seconds to move it to a constant.Context
StackExchange Code Review Q#107625, answer score: 3
Revisions (0)
No revisions yet.