patternjavaMinor
AsyncTask method
Viewed 0 times
asynctaskmethodstackoverflow
Problem
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
```
private class TabNameSync extends AsyncTask {
String BASE_URL = Config.DATA_URL;
ProgressDialog nDialog;
HashMap 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();
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 {
String BASE_URL = Config.DATA_URL;
ProgressDialog nDialog;
HashMap 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();
Solution
OK, so I'm probably being quite picky here, but:
- Your definition of
hashMapPostshould be changed to... = new HashMap();.hashMapPostisn't a good name for the variable. You probably also shouldn't be using aHashMap- considerHttpPost.
item1isn't a good name for the variable. This is never actually used, despite being initialised in a loop, delete it.
meisn'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.
bufferVariableisn'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.dbUtilis 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,
editorandeditor1- this should at least beeditor1andeditor2- 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).
Context
StackExchange Code Review Q#127297, answer score: 3
Revisions (0)
No revisions yet.