2
\$\begingroup\$

This class retrieves JSON data from a PHP script and uses the data to populate a ListView using AsyncTasks. Any ideas on how to improve it?

public class MainActivity extends ActionBarActivity {

    ArrayList<Location> arrayOfLocations;
    LocationAdapter adapter;

    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_main);

        // Construct the data source
        arrayOfLocations = new ArrayList<Location>();

        // Create the adapter to convert the array to views
        adapter = new LocationAdapter(this, arrayOfLocations);

        FillLocations myFill = new FillLocations();
        myFill.execute();

    }

        //this class downloads an image and passes that (and other data) into an
        //adapter which populates the ListView
    class BitmapWorkerTask extends AsyncTask<Integer, Void, Bitmap> {
        private int photoID = 1;
        JSONObject json = null;

        public BitmapWorkerTask(int photoID, JSONObject json) {
            // Use a WeakReference to ensure the ImageView can be garbage
            // collected
            this.photoID = photoID;
            this.json = json;
        }

        protected void onPreExecute() {

        }

        // Decode image in background.
        @Override
        protected Bitmap doInBackground(Integer... params) {
            String initialURL = "http://afs.spotcontent.com/img/Places/Icons/";
            final String updatedURL = initialURL + photoID + ".jpg";
            Bitmap bitmap2 = null;

            try {
                bitmap2 = BitmapFactory.decodeStream((InputStream) new URL(
                        updatedURL).getContent());

            } catch (MalformedURLException e) {
                e.printStackTrace();
            } catch (IOException e) {
                e.printStackTrace();
            }

            return bitmap2;
        }

        // Once complete, see if ImageView is still around and set bitmap.
        @Override
        protected void onPostExecute(Bitmap bitmap2) {
            try {
                adapter.add(new Location(bitmap2, json.getString("PlaceTitle"),
                        json.getString("PlaceDetails"), json
                                .getString("PlaceDistance"), json
                                .getString("PlaceUpdatedTime")));
            } catch (JSONException e) {
                // TODO Auto-generated catch block
                e.printStackTrace();
            }

        }
    }

        //this class retrieves the JSON data from the PHP script
        //and parses it, then it calls the Bitmap task to get the
        //image and then add the location
    private class FillLocations extends AsyncTask<Integer, Void, String> {

        protected void onPreExecute() {
        }

        // Decode image in background.
        @Override
        protected String doInBackground(Integer... params) {

            String result = "";
            InputStream isr = null;
            try {
                HttpClient httpclient = new DefaultHttpClient();
                HttpPost httppost = new HttpPost("http://myscript.php/"); // YOUR
                                                                                    // PHP
                                                                                    // SCRIPT
                                                                                    // ADDRESS
                HttpResponse response = httpclient.execute(httppost);
                HttpEntity entity = response.getEntity();
                isr = entity.getContent();
                // resultView.setText("connected");
            } catch (Exception e) {
                Log.e("log_tag", "Error in http connection " + e.toString());
            }
            // convert response to string
            try {
                BufferedReader reader = new BufferedReader(
                        new InputStreamReader(isr, "iso-8859-1"), 8);
                StringBuilder sb = new StringBuilder();
                String line = null;
                while ((line = reader.readLine()) != null) {
                    sb.append(line + "\n");
                }
                isr.close();

                result = sb.toString();
            } catch (Exception e) {
                Log.e("log_tag", "Error  converting result " + e.toString());
            }

            // parse json data
            try {
                JSONArray jArray = new JSONArray(result);

                for (int i = 0; i < jArray.length(); i++) {
                    final JSONObject json = jArray.getJSONObject(i);
                    counter++;

                    try {

                        BitmapWorkerTask myTask = new BitmapWorkerTask(
                                json.getInt("ID"), json);
                        myTask.execute();

                    } catch (JSONException e) {
                        // TODO Auto-generated catch block
                        e.printStackTrace();
                    }

                }

            } catch (Exception e) {
                // TODO: handle exception
                Log.e("log_tag", "Error Parsing Data " + e.toString());
            }
            return msg;

        }

        protected void onPostExecute(String msg) {
            // Attach the adapter to a ListView
            ListView listView = (ListView) findViewById(R.id.listView1);

            listView.setAdapter(adapter);

        }
    }
}
\$\endgroup\$
  • \$\begingroup\$ You mention php in the title, in your post and in the code comments. It would probably be clearer if you did not talk about php at all and just wrote about retrieving json data. \$\endgroup\$ – toto2 Jul 10 '14 at 17:52
2
\$\begingroup\$

Well, depending on what SDK level you need to support, you could use HttpUrlConnection instead of the deprecated DefaultHttpClient.

Quoting from a post in the Android blog:

"Apache HTTP client has fewer bugs on Eclair and Froyo. It is the best choice for these releases.

For Gingerbread and better, HttpURLConnection is the best choice. Its simple API and small size makes it great fit for Android. Transparent compression and response caching reduce network use, improve speed and save battery. New applications should use HttpURLConnection; it is where we will be spending our energy going forward."

Source: http://android-developers.blogspot.ca/2011/09/androids-http-clients.html

\$\endgroup\$
1
\$\begingroup\$

I only have some minor comments:

  • I would change the name of FillLocations to LocationsFetchingTask. Similarly, I would change BitmapWorkerTask to BitmapFetchingTask, but at least you kept the Task suffix for that one.

  • It's not clear why you set the adapter in FillLocations.onPostExecute. You could as well have done it at the start in onCreate. I don't see any difference except that setting it at the start is clearer. When FillLocations.onPostExecute is called, it is very likely that adapter is still completely empty since the BitmapWorkerTasks have most likely not terminated.

  • Your methods are way too long. If you need to do many steps in a call to doInBackground(), you can break those steps in a few methods and define those methods in the current AsyncTask.

\$\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.