3
\$\begingroup\$

I wrote a login system for an app I am developing, but I'm not really happy with the code I have at the moment. I feel like there is a better way at tackling a problem like this. In particular I'm not happy with the way my callback method has to be called specifically on the ui thread, I thought maybe there is a better way of doing that. I have 2 fields; username and password

  • The user presses the login button
  • A new thread is started that handles the connecting to a web page
  • The webpage tries to find a row that matches with the username and password
  • The page outputs a 0 on success and 1 on failure
  • The app calls the callback with the output of the webpage
  • The callback runs on the main thread and decides what to do (next activity or a Toast)

These are the most important methods in my LoginActivity class

private boolean emptyField() {
        if (usernameEt.getText().toString().isEmpty()) {
            Toast.makeText(getApplicationContext(), "Please fill in a username", Toast.LENGTH_SHORT).show();
            return true;
        }
        if (passwordEt.getText().toString().isEmpty()) {
            Toast.makeText(getApplicationContext(), "Please fill in a password", Toast.LENGTH_SHORT).show();
            return true;
        }
        return false;
    }

    private void login() {
        if (emptyField()) {
            return;
        }
        setEnabled(false);
        new Login().makeRequest(new Callback() {
            @Override
            public void onComplete(Result result) {
                setEnabled(true);
                if (result instanceof Result.Success) {
                    Intent dsp = new Intent(LoginActivity.this, MainActivity.class);
                    startActivity(dsp);
                } else {
                    Result.Error error = (Result.Error) result;
                    Toast.makeText(getApplicationContext(), error.message, Toast.LENGTH_SHORT).show();
                }
            }
        }, usernameEt.getText().toString(), passwordEt.getText().toString());
    }

    private void setEnabled(boolean enabled) {
        usernameEt.setEnabled(enabled);
        passwordEt.setEnabled(enabled);
        buttonLogin.setEnabled(enabled);
    }

The login class that handles the complete login sequence

public class Login {
    public void makeRequest(final Callback callback, final String username, final String password) {
        new Thread(new Runnable() {
            @Override
            public void run() {
                final Result result = makeSynchronousRequest(username, password);
                callback.execute(result);
            }
        }).start();
    }

    public Result makeSynchronousRequest(String username, String password) {
        String urlString = "http://10.0.2.2/login.php";
        try {
            URL url = new URL(urlString);
            URLConnection connection = url.openConnection();
            String data = URLEncoder.encode("name", "UTF-8") + "=" + URLEncoder.encode(username, "UTF-8") +
                    "&&" + URLEncoder.encode("pass", "UTF-8") + "=" + URLEncoder.encode(password, "UTF-8");
            connection.setDoOutput(true);
            connection.setConnectTimeout(5000);

            try (OutputStreamWriter wr = new OutputStreamWriter(connection.getOutputStream())) {
                wr.write(data);
            }
            try (BufferedReader reader = new BufferedReader(new InputStreamReader(connection.getInputStream()))) {
                StringBuilder sb = new StringBuilder();
                String line;

                // Read Server Response
                while ((line = reader.readLine()) != null) {
                    sb.append(line);
                }
                String returnData = sb.toString();
                switch (Integer.parseInt(returnData)) {
                    case 0:
                        return new Result.Success<>("");
                    case 1:
                        return new Result.Error("Username and password do not match");
                }
                return new Result.Error("Something unexpected happened: " + returnData);
            }
        } catch (SocketTimeoutException e) {
            return new Result.Error("The login page is currently offline");
        } catch (IOException e) {
            e.printStackTrace();
            return new Result.Error(e.getMessage());
        }
    }
}

Result class

public abstract class Result {

    private Result() {}

    public static final class Success<T> extends Result {
        public T data;

        public Success(T data) {
            this.data = data;
        }
    }

    public static final class Error extends Result {
        public String message;

        public Error(String message) {
            this.message = message;
        }
    }

}

Callback class

public abstract class Callback {

    public abstract void onComplete(Result result);

    public void execute(final Result result) {
        new Handler(Looper.getMainLooper()).post(new Runnable() {
            @Override
            public void run() {
                onComplete(result);
            }
        });
    }
}
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

I have read your question since a few days. Given that nobody else has something to say, you can guess that your code is quite good.

I do not develop on Android so I cannot say anything about your concern of the way the callback method has to be called specifically on the ui thread. However I can give you some hints on your code:

Be coherent and use meaningful method names; emptyField looks like it will clear the fields while it will validate them and display a message. You should rename it to something more clear on what it will do.

Apply the Single responsibility principle. The emptyField method validate and display a message. Try to create two separate methods, one to validate and another to display the errors. By doing this you will hopefully create one method that display all the errors at once, this is better for the user experience.

The Single responsibility principle also apply to your Login class. Try to move all the code used to communicate with the server to another class. Keep only the formatting and parsing logic in the Login so that you can easily create other kind of "requests" later without duplicating the communication code. There are libraries aimed to communicate with a server, you should consider using one if you plan to send more requests to your server.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.