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();
}
}
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 aHashMap
- considerHttpPost
. 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 fromdoInBackground()
. I'd suggest, at a minimum, something along the lines ofprepareRequest()
,createConnection()
,sendData(conn, req)
andparseResponse(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 likeresponseData
.- 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
andeditor1
- this should at least beeditor1
andeditor2
- but the names should be more descriptive about what they're editing. onPostExecute()
method body is too long. Refactor along similar comment fordoInBackground()
- 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– Marc-Andre2016年05月06日 21:44:22 +00:00Commented 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– Dave2016年05月06日 22:21:30 +00:00Commented May 6, 2016 at 22:21
SQLiteOpenHelper
. Then call it from your Activities. \$\endgroup\$