patternjavaMinor
Generic HTTP using Android Asynctask
Viewed 0 times
genericandroidhttpusingasynctask
Problem
I have developed a generic HTTP functionality using Android Asynctask and Apache HTTP client. Please review the code and let me know if it is the right way of doing or there are other ways to achieve it.
```
public class HTTPAsyncTask extends AsyncTask {
private CallBack mCb;
HashMap mData = null;
List mParams= new ArrayList();
String mTypeOfRequest;
String mStrToBeAppended = "";
boolean isPostDataInJSONFormat = false;
JSONObject mJSONPostData = null;
public HTTPAsyncTask(CallBack c, HashMap data, JSONObject jsonObj, String request) {
mCb = c;
mTypeOfRequest = request;
mJSONPostData = jsonObj;
if((data != null) && (jsonObj == null)){
mData = data;
if(mTypeOfRequest.equalsIgnoreCase("GET")){
Iterator it = mData.keySet().iterator();
while(it.hasNext()){
String key = it.next();
mParams.add(new BasicNameValuePair(key, mData.get(key)));
}
for (int i = 0; i it = mData.keySet().iterator();
while(it.hasNext()){
String key = it.next();
mParams.add(new BasicNameValuePair(key, mData.get(key)));
}
}
}
if ((mData == null) && (jsonObj != null)){
isPostDataInJSONFormat = true;
}
}
@Override
protected String doInBackground(String... baseUrls) {
publishProgress(null);
if(mTypeOfRequest.equalsIgnoreCase("GET")){
String finalURL = baseUrls[0]+ mStrToBeAppended;
return HttpUtility.GET(finalURL);
}
if (mTypeOfRequest.equalsIgnoreCase("POST")){
if(isPostDataInJSONFormat == false){
return HttpUtility.POST(baseUrls[0],mParams );
}
else {
return HttpUtility.POST(baseUrls[0], mJSONPostData);
}
Async class```
public class HTTPAsyncTask extends AsyncTask {
private CallBack mCb;
HashMap mData = null;
List mParams= new ArrayList();
String mTypeOfRequest;
String mStrToBeAppended = "";
boolean isPostDataInJSONFormat = false;
JSONObject mJSONPostData = null;
public HTTPAsyncTask(CallBack c, HashMap data, JSONObject jsonObj, String request) {
mCb = c;
mTypeOfRequest = request;
mJSONPostData = jsonObj;
if((data != null) && (jsonObj == null)){
mData = data;
if(mTypeOfRequest.equalsIgnoreCase("GET")){
Iterator it = mData.keySet().iterator();
while(it.hasNext()){
String key = it.next();
mParams.add(new BasicNameValuePair(key, mData.get(key)));
}
for (int i = 0; i it = mData.keySet().iterator();
while(it.hasNext()){
String key = it.next();
mParams.add(new BasicNameValuePair(key, mData.get(key)));
}
}
}
if ((mData == null) && (jsonObj != null)){
isPostDataInJSONFormat = true;
}
}
@Override
protected String doInBackground(String... baseUrls) {
publishProgress(null);
if(mTypeOfRequest.equalsIgnoreCase("GET")){
String finalURL = baseUrls[0]+ mStrToBeAppended;
return HttpUtility.GET(finalURL);
}
if (mTypeOfRequest.equalsIgnoreCase("POST")){
if(isPostDataInJSONFormat == false){
return HttpUtility.POST(baseUrls[0],mParams );
}
else {
return HttpUtility.POST(baseUrls[0], mJSONPostData);
}
Solution
Use a code formatter, one is probably included in your editor. [ctrl+shift+f] in eclipse
Avoid setting variables to null. This turns off warnings and errors and allows you to make mistakes otherwise not possible!
Declare variables as late as possible.
Use final where applicable,
Strings can contain anything. But you have only 2 values (get/post). Consider using a boolean useGet.
Constructor and methods checks ("if") same value. Consider using different classes. One for each "if" outcome. One for get and one for post.
Move the constructor logic to separate method static "constructor" method to ease testing (easy to create object with different contents).
Return as soon as you have done what you want to do. Saving the value for later is confusing at best. All good editors will highlight all exit paths from
Don't return garbage data where you expect good data. Change
to
Avoid setting variables to null. This turns off warnings and errors and allows you to make mistakes otherwise not possible!
Declare variables as late as possible.
Use final where applicable,
final String mTypeOfRequest;Strings can contain anything. But you have only 2 values (get/post). Consider using a boolean useGet.
Constructor and methods checks ("if") same value. Consider using different classes. One for each "if" outcome. One for get and one for post.
Move the constructor logic to separate method static "constructor" method to ease testing (easy to create object with different contents).
Return as soon as you have done what you want to do. Saving the value for later is confusing at best. All good editors will highlight all exit paths from
Don't return garbage data where you expect good data. Change
result = "Did not work!";to
return ""; or throw a checked exceptionCode Snippets
final String mTypeOfRequest;result = "Did not work!";return ""; or throw a checked exceptionContext
StackExchange Code Review Q#53768, answer score: 4
Revisions (0)
No revisions yet.