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

Login UI for an Android app

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
loginforandroidapp

Problem

This code starts a new thread for login and reacts according to the JSON result returned by the server.

I think this code has too many conditionals, exception handlers, and nested functions.

```
public void Login() {
mButton.setClickable(false);
mButton.setText("Loading...");
final String username = mUserName.getText().toString().trim();
final String password = mPassWord.getText().toString().trim();

new Thread() {
public void run() {
final String result = NetUtil.loginByGet(username, password);
if (result != null) {
try {
JSONTokener jsonTokener = new JSONTokener(result);
JSONObject jsonObject = (JSONObject) jsonTokener.nextValue();
final String message = jsonObject.getString("message");
if (jsonObject.getInt("error") == 0) {
String token = jsonObject.getString("token");
boolean isSaveSuccess = InfoUtil.saveUserInfo(LoginActivity.this,token);
if (isSaveSuccess) {
runOnUiThread(new Runnable() {
@Override
public void run() {
Toast.makeText(LoginActivity.this, "登录成功", Toast.LENGTH_SHORT).show();
initIntent(MainActivity.class);
finish();
}
});
}else {
runOnUiThread(new Runnable() {
@Override
public void run() {
Toast.makeText(LoginActivity.this, "保存登录信息失败", Toast.LENGTH_SHORT).show();
mButton.setClickable(true);
mButton.setText("登录");
}

Solution

Using an AsyncTask instead would be much better. You've got many callbacks to the UI thread, all of this you could do in the post execute method instead:

private class LoginTask extends AsyncTask{

    private String username;
    private String password;
    private Exception exception;
    private boolean isSaveSuccess;

    public LoginTask(String username, String password){
        this.username = username;
        this.password = password;
    }

    @Override
    protected String doInBackground(Void... voids) {
        try{
            String result = NetUtil.loginByGet(username, password);
            //Grab the token
            isSaveSuccess = InfoUtil.saveUserInfo(LoginActivity.this,token);
        }catch(Exception ex){
            //Catch any given exception to use it in the post execution (e.g. show a toast)
            this.exception = ex;
        }
    }

    @Override
    protected void onPostExecute(String result) {
        //Here it goes your UI logic
    }

}


To call it, just launch an execution when login button is clicked:

public void doLogin(){    
    mButton.setClickable(false);
    mButton.setText("Loading...");
    String username = mUserName.getText().toString().trim();
    String password = mPassWord.getText().toString().trim();
    new LoginTask(username, password).execute();
}

Code Snippets

private class LoginTask extends AsyncTask<Void, Void, String>{

    private String username;
    private String password;
    private Exception exception;
    private boolean isSaveSuccess;

    public LoginTask(String username, String password){
        this.username = username;
        this.password = password;
    }

    @Override
    protected String doInBackground(Void... voids) {
        try{
            String result = NetUtil.loginByGet(username, password);
            //Grab the token
            isSaveSuccess = InfoUtil.saveUserInfo(LoginActivity.this,token);
        }catch(Exception ex){
            //Catch any given exception to use it in the post execution (e.g. show a toast)
            this.exception = ex;
        }
    }

    @Override
    protected void onPostExecute(String result) {
        //Here it goes your UI logic
    }

}
public void doLogin(){    
    mButton.setClickable(false);
    mButton.setText("Loading...");
    String username = mUserName.getText().toString().trim();
    String password = mPassWord.getText().toString().trim();
    new LoginTask(username, password).execute();
}

Context

StackExchange Code Review Q#126840, answer score: 7

Revisions (0)

No revisions yet.