3
\$\begingroup\$

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;
 @Override
 protected void onPreExecute() {
 nDialog = new ProgressDialog(MainActivity.this);
 nDialog.setMessage("Loading...");
 nDialog.setIndeterminate(true);
 nDialog.setCancelable(true);
 nDialog.show();
 }
 @Override
 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();
 con.setDoInput(true);
 con.setDoOutput(true);
 con.setRequestMethod("POST");
 con.connect();
 OutputStream os = con.getOutputStream();
 BufferedWriter writer = new BufferedWriter(
 new OutputStreamWriter(os, "UTF-8"));
 writer.write(commonUtil.getPostDataString(hashMapPost));
 writer.flush();
 writer.close();
 os.close();
 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)
 buffer.append(line).append("\r\n");
 is.close();
 }
 con.disconnect();
 bufferVariable = buffer.toString();
 return buffer.toString();
 } catch (Throwable t) {
 t.printStackTrace();
 } finally {
 try {
 if (is != null) {
 is.close();
 }
 } catch (Throwable t) {
 }
 try {
 if (con != null) {
 con.disconnect();
 }
 } catch (Throwable t) {
 }
 if (!bufferVariable.equalsIgnoreCase(" ")) {
 try {
 JSONArray jArray = new JSONArray(bufferVariable);
 int jsonLength = jArray.length();
 commonUtil.dbUtil.open();
 commonUtil.dbUtil.resetAddNew();
 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,
 jsonObj.getString("dbname"));
 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)
 CatId.add(strCatId);
 Image.add(strImage);
 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);
 editor1.commit();
 }
 Set<String> set = new HashSet<String>();
 set.addAll(Image);
 SharedPreferences.Editor editor = commonUtil.pref.edit();
 editor.putStringSet("sharedImageID", set);
 editor.commit();
 }
 } catch (JSONException e) {
 e.printStackTrace();
 }
 }
 }
 return null;
 }
 @Override
 protected void onPostExecute(String result) {
 super.onPostExecute(result);
 final TabLayout tabLayout = (TabLayout) findViewById(R.id.tabs);
 tabLayout.setTabMode(TabLayout.MODE_SCROLLABLE); 
/* Getting data from DB */
 CommonUtil.dbUtil.open();
 curTabName = CommonUtil.dbUtil.getTabName();
 if (curTabName != null && curTabName.moveToFirst()) {
 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)) {
 jSonTab.add(tabName);
 }
 if (!jSonCatId.contains(tabId)) {
 jSonCatId.add(tabId);
 }
 if (!jSonImgId.contains(image)) {
 jSonImgId.add(image);
 }
 curTabName.moveToNext();
 }
 tabName = jSonTab;
 CatId = jSonCatId;
 Set<String> set = new HashSet<String>();
 set.addAll(jSonImgId);
 SharedPreferences.Editor editor = commonUtil.pref.edit();
 editor.putStringSet("sharedImageID", set);
 editor.commit();
 }
 nDialog.dismiss();
 }
 }
asked May 2, 2016 at 13:23
\$\endgroup\$
2
  • \$\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\$ Commented 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\$ Commented May 6, 2016 at 17:39

1 Answer 1

3
+50
\$\begingroup\$

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).
answered May 6, 2016 at 21:11
\$\endgroup\$
2
  • \$\begingroup\$ I'm not sure try-with-resources is available in Android, but I never done any Android development either. \$\endgroup\$ Commented 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\$ Commented May 6, 2016 at 22:21

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.