HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Generic HTTP using Android Asynctask

Submitted by: @import:stackexchange-codereview··
0
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.

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,

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 exception

Code Snippets

final String mTypeOfRequest;
result = "Did not work!";
return ""; or throw a checked exception

Context

StackExchange Code Review Q#53768, answer score: 4

Revisions (0)

No revisions yet.