Even I know that this isn't a good way of writing code, but I need to improve this.

Here I am retrieving data from Server in Json format by Posting some variables in doInbackground. Getting all the products depends on Posting variable ie Latitude and Longitude of Current Location.All the Json data are stored in LocalDB and for accessing it I used Object Class. Depends on the users current location, the nearby restaurant will be delivered the food once they ordered. Now the Dummy data has 101 products and 13 category.So the category is added ArrayList as a duplicates category id which will be repeated whenever the product is added. So I wrote Set< > code for removing the duplicates.Because these sorted category will be set in the Tabs as a TabTitleName of it.

     private class TabNameSync extends AsyncTask<Void, Void, String> {
            String BASE_URL = Config.DATA_URL;
            ProgressDialog nDialog;
            HashMap<String, String> hashMapPost = new HashMap<>();
            CartRes item1 = new CartRes();              // Object Class initialization
            Map.Entry me;

            protected void onPreExecute() {
                nDialog = new ProgressDialog(MainActivity.this);

            protected String doInBackground(Void... params) {

                HttpURLConnection con = null;
                InputStream is = null;

                StringBuffer buffer;
                String bufferVariable = null;

                hashMapPost.put("tag", "onload");                     // values to get the response by Posting
                hashMapPost.put("lat", "8.7xxxxxxxx");          
                hashMapPost.put("log", "77.7xxxxxxx");

                try {
                    con = (HttpURLConnection) (new URL(BASE_URL)).openConnection();
                    OutputStream os = con.getOutputStream();
                    BufferedWriter writer = new BufferedWriter(
                            new OutputStreamWriter(os, "UTF-8"));


                    buffer = new StringBuffer();
                    int responseCode = con.getResponseCode();
                    if (responseCode == HttpsURLConnection.HTTP_OK) {
                        is = con.getInputStream();
                        BufferedReader br = new BufferedReader(new InputStreamReader(is));
                        String line;
                        while ((line = br.readLine()) != null)
                    bufferVariable = buffer.toString();
                    return buffer.toString();
                } catch (Throwable t) {
                } finally {
                    try {
                        if (is != null) {
                    } catch (Throwable t) {
                    try {
                        if (con != null) {
                    } catch (Throwable t) {
                    if (!bufferVariable.equalsIgnoreCase(" ")) {
                        try {
                            JSONArray jArray = new JSONArray(bufferVariable);
                            int jsonLength = jArray.length();
                            for (int j = 0; j < jArray.length(); j++) {
                                item1 = new CartRes();
                                JSONObject jsonObj = jArray.getJSONObject(j);
                                String ShopID = "100";
                                Double MostSum = jsonObj.getDouble(Config.MOST_SUM);
                                int mostSum = (int) Math.round(MostSum);
    /*  Storing data retrieved Data into DB */
                                commonUtil.dbUtil.addProductList(jsonObj.getInt(Config.CATEID), jsonObj.getString(Config.CATENAME),
                                        jsonObj.getInt(Config.PRODUCTID), jsonObj.getString(Config.PRODUCTNAME),
                                        jsonObj.getString(Config.IMGID), jsonObj.getString(Config.SALESPRICE),
                                        jsonObj.getString(Config.VOUCHERID), jsonObj.getString(Config.VOUCHEROFFER),
                                        jsonObj.getInt(Config.LIKE), jsonObj.getInt(Config.DELIVERYTIME),
                                        mostSum, ShopID,

                                strTabName = jsonObj.getString("Cate Name");
                                int strCatId = jsonObj.getInt("Cat Id");
                                strDbName = jsonObj.getString("dbname");
                                String strImage = jsonObj.getString("Image");

                    tabName.add(strTabName);  //Storing category values into the ArrayList (tabName)

                                if (jsonLength == jArray.length()) {
                                    JSONObject jsonObj2 = jArray.getJSONObject(jsonLength - 1);
                                    Config.IMAGE_URL = jsonObj2.getString("url");
                                    SharedPreferences.Editor editor1 = CommonUtil.pref.edit();
                                    editor1.putString("URL", Config.IMAGE_URL);
                                Set<String> set = new HashSet<String>();
                                SharedPreferences.Editor editor = commonUtil.pref.edit();
                                editor.putStringSet("sharedImageID", set);
                        } catch (JSONException e) {
                return null;

            protected void onPostExecute(String result) {

                final TabLayout tabLayout = (TabLayout) findViewById(R.id.tabs);

/*  Getting data from DB */
                curTabName = CommonUtil.dbUtil.getTabName();
                if (curTabName != null && curTabName.moveToFirst()) {
                    for (int i = 0; i < curTabName.getCount(); i++) {
                        String tabName = curTabName.getString(curTabName.getColumnIndex(DbHelper.JSON_CATEGORY_NAME));
                        int tabId = curTabName.getInt(curTabName.getColumnIndex(DbHelper.JSON_CATEGORY_ID));
                        String pdtName = curTabName.getString(curTabName.getColumnIndex(DbHelper.JSON_PRODUCT_NAME));
                        int pdtId = curTabName.getInt(curTabName.getColumnIndex(DbHelper.JSON_PRODUCT_ID));
                        int pdtPrice = curTabName.getInt(curTabName.getColumnIndex(DbHelper.JSON_SALES_PRICE));
                        String shopId = curTabName.getString(curTabName.getColumnIndex(DbHelper.JSON_SHOP_ID));
                        String image = curTabName.getString(curTabName.getColumnIndex(DbHelper.JSON_IMAGE_ID));
                        int likeCount = curTabName.getInt(curTabName.getColumnIndex(DbHelper.JSON_LIKECOUNT));
                        String delivery = curTabName.getString(curTabName.getColumnIndex(DbHelper.JSON_DELIVERY_TIME));
                        String voucherOffer = curTabName.getString(curTabName.getColumnIndex(DbHelper.JSON_VOUCHER_OFFER));
                        String voucherId = curTabName.getString(curTabName.getColumnIndex(DbHelper.JSON_VOUCHER_ID));
                        float MostSum = curTabName.getFloat(curTabName.getColumnIndex(DbHelper.JSON_MOST_SUM));
                        int intMOST;
                        intMOST = (int) MostSum;
    //Adding all retrieved data into Object Class
                        cartRestaurant.add(new CartRes(tabId, tabName, pdtId, pdtName, pdtPrice, image, shopId, delivery, likeCount, voucherId, voucherOffer, intMOST));
    /*  Having 101 products right now, so category retrieve as duplicate.To avoid it, use the following */
                        if (!jSonTab.contains(tabName)) {
                        if (!jSonCatId.contains(tabId)) {
                        if (!jSonImgId.contains(image)) {
                    tabName = jSonTab;
                    CatId = jSonCatId;

                    Set<String> set = new HashSet<String>();

                    SharedPreferences.Editor editor = commonUtil.pref.edit();
                    editor.putStringSet("sharedImageID", set);
  • \$\begingroup\$ There is a lot to refactor in this code. I would go with couple of suggestions to start, first of all, don't manage HTTP connections yourself. Use android volley or retrofit which have much nicer APIs. The same for DB methods, create a class for managing all the DB related operations and make it extend SQLiteOpenHelper. Then call it from your Activities. \$\endgroup\$
    – Aritz
    May 6, 2016 at 15:52
  • \$\begingroup\$ @XtremeBiker This look more like an answer than a comment. Short answer are still valid on Code Review. \$\endgroup\$
    – Marc-Andre
    May 6, 2016 at 17:39

1 Answer 1


OK, so I'm probably being quite picky here, but:

  • Your definition of hashMapPost should be changed to ... = new HashMap<String, String>();. hashMapPost isn't a good name for the variable. You probably also shouldn't be using a HashMap - consider HttpPost.
  • item1 isn't a good name for the variable. This is never actually used, despite being initialised in a loop, delete it.
  • me isn't a good name for the variable. It's not actually used in the code provided. Delete it.
  • doInBackground() method body is far too long, refactor to re-usable methods and call these from doInBackground(). I'd suggest, at a minimum, something along the lines of prepareRequest(), createConnection(), sendData(conn, req) and parseResponse(resp).
  • Consider using try-with-resources instead of finally {...} blocks. try-with-resources will automatically close your resources when finished with them.
  • bufferVariable isn't a good name for the variable. Use something like responseData.
  • Consider doing something if the HTTP response code is not HTTP_OK. Maybe informing the user is appropriate?
  • As you're iterating over arrays, use the Iterable interface properly. I.e. for(Object o : arr) rather than using the length of the array directly.
  • commonUtil.dbUtil is bad practice. Consider using a more appropriate persistence method. Things like Hibernate and JPA may be overkill, but putting utilities in "common" classes/packages is generally discouraged.
  • Refactor the JSON parsing, build some Object from the JSON and pass this along for storage in the DB.
  • You have two editors, editor and editor1 - this should at least be editor1 and editor2 - but the names should be more descriptive about what they're editing.
  • onPostExecute() method body is too long. Refactor along similar comment for doInBackground()
  • When exceptions happen, display some appropriate text to the user. Printing stacktraces is only meaningful to developers.
  • There's no point initialising variables to null (that's the default).
  • \$\begingroup\$ I'm not sure try-with-resources is available in Android, but I never done any Android development either. \$\endgroup\$
    – Marc-Andre
    May 6, 2016 at 21:44
  • 2
    \$\begingroup\$ try-with-resources works with API level 13 & above, though AndroidStudio will complain about it and tell you that you need to be using API level 19 or above. See this for some discussion: code.google.com/p/android/issues/detail?id=73483 \$\endgroup\$
    – Dave
    May 6, 2016 at 22:21

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.