Multiple background tasks

This is taken from my post at Stack Overflow here.

I need your help to review my code for improvement and best practice.

```
private void logSessionToServer() {
// TODO Auto-generated method stub

Thread thread = new Thread(){
public void run(){
strUsername = etUsername.getText().toString();

// Building Parameters
List params = new ArrayList();
params.add(new BasicNameValuePair( TAG_USERNAME, strUsername ));

// getting JSON Object
// Note it accepts POST method only
JSONObject json = jsonParser.makeHttpRequest(url_log_session,
"POST", params);

// check log cat from response
Log.d("Create Response", json.toString());

// check for success tag
try {
int success = json.getInt(TAG_SUCCESS);

if (success == 1) {

Log.d("", "Successfully logged session!");

} else {
// failed
Log.d("", "Not Successful!");
}
} catch (JSONException e) {
e.printStackTrace();
}
}
};

thread.start();
}

/**
* Background Async Task to Create new record
/
class LoginTask extends AsyncTask {

/**
* Before starting background thread Show Progress Dialog
/
protected void onPreExecute() {
super.onPreExecute();
pDialog = new ProgressDialog(Login.this);
pDialog.setIcon(R.drawable.upload);
pDialog.setTitle("Connecting to server");
pDialog.setMessage("Validating login details..");
pDialog.setIndeterminate(false);
pDialog.setCancelable(true);
pDialog.show();
}

/**
*
/
protected String doInBackground(String... args) {

strUsername = etUsername.getText().toString();
strPassword = etPassword.getText().toString();

// Buildin

Have descriptive, intuitive variable names. (Example: what is etUsername? No clue.)

You have a List of a NameValuePair. It looks like the your custom NameValuePair class is much like a key-value pair in a Map. I think it would simplify your code a lot if you just went ahead and used HashMap instead of jumping through needless hoops.

Be more consistent in your naming conventions. You have some variables which are named in camelCase (which is the accepted way to do it in Java), and then you have some which have the elements separated by underscores (url_log_session). The capitalized/underscore format is good for constants (like your TAG_USERNAME), but that's it.

It's good that you're trying to comment your code, but some of your comments are overkill and unnecessary. Because you chose good names for your methods, the code is self-documenting in some places, which is a good thing! So the following comments are pretty much useless, as they say exactly what the method names express:

saveUserInfoToDB(); // Save user info to DB
saveUserloggedIn(); // Save user logged in to preference
savePriestInfoToDB(); // Save priest to DB

When dealing with Threads, you should almost always have the run() functionality defined in some class that implements Runnable rather than extending Thread itself. So you can have an anonymous class or an explicit one. The standard way to do it in-line like you tried to is like this:

Thread thread = new Thread(new Runnable() {
    @Override
    public void run() {
        // do stuff
    }
});

If you don't need the reference to the thread, you can even do it more simply, like the following:

new Thread(new Runnable() {
    @Override
    public void run() {
        // do stuff
    }
}).start();

StackExchange Code Review Q#51503, answer score: 6