Logging in to Android application using Firebase

Question

I currently have an app that uses Firebase for logging users in and I would like to know if I can make this code any better. I currently have 4 files:

auth/models/User.java

auth/BaseActivity.java

auth/CreateAccountActivity.java

auth/LoginActivity.java

User.java:

@IgnoreExtraProperties
public class User {

    public String username;
    public String email;

    public User() {

    }

    public User(String username, String email) {
        this.username = username;
        this.email = email;
    }

}

BaseActivity.java

public class BaseActivity extends AppCompatActivity {

    private ProgressDialog mProgressDialog;

    public void showProgressDialog() {
        if (mProgressDialog == null) {
            mProgressDialog = new ProgressDialog(this);
            mProgressDialog.setMessage(getString(R.string.loading));
            mProgressDialog.setIndeterminate(true);
            mProgressDialog.setCancelable(false);
        }
        mProgressDialog.show();
    }

    public void hideProgressDialog() {
        if (mProgressDialog != null && mProgressDialog.isShowing()) {
            mProgressDialog.dismiss();
        }
    }

    @Override
    public void onDestroy() {
        super.onDestroy();
        hideProgressDialog();
    }

}

CreateAccountActivity.java:

public class CreateAccountActivity extends BaseActivity {

    private EditText mUsernameField, mEmailField, mPasswordField;

    private DatabaseReference mDatabase;

    private FirebaseAuth mAuth;

    private FirebaseAuth.AuthStateListener mAuthListener;

    ProgressDialog mProgressDialog;

    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_create_account);

        mUsernameField = (EditText) findViewById(R.id.createAccountUsernameInput);
        mEmailField = (EditText) findViewById(R.id.createAccountEmailInput);
        mPasswordField = (EditText) findViewById(R.id.createAccountPasswordInput);

        Button mCreateAccountButton = (Button) findViewById(R.id.createAccountButton);

        TextView mLoginLink = (TextView) findViewById(R.id.loginLink);

        mProgressDialog = new ProgressDialog(this);
        mProgressDialog.setCancelable(false);
        mProgressDialog.setMessage("Please wait...");

        mDatabase = FirebaseDatabase.getInstance().getReference();
        mAuth = FirebaseAuth.getInstance();

        mAuthListener = new FirebaseAuth.AuthStateListener() {
            @Override
            public void onAuthStateChanged(@NonNull FirebaseAuth firebaseAuth) {
                FirebaseUser user = firebaseAuth.getCurrentUser();
                if (user != null) {
                    startActivity(new Intent(CreateAccountActivity.this, MainActivity.class));
                    finish();
                }
            }
        };

        assert mCreateAccountButton != null;
        mCreateAccountButton.setOnClickListener(new View.OnClickListener() {
            @Override
            public void onClick(View v) {
                createAccount(mEmailField.getText().toString(), mPasswordField.getText().toString());
            }
        });

        assert mLoginLink != null;
        mLoginLink.setOnClickListener(new View.OnClickListener() {
            @Override
            public void onClick(View v) {
                startActivity(new Intent(CreateAccountActivity.this, LoginActivity.class));
                finish();
            }
        });
    }

    public void createAccount(String email, String password) {
        if (!validateForm()) {
            return;
        }

        showProgressDialog();

        mAuth.createUserWithEmailAndPassword(email, password).addOnCompleteListener(this, new OnCompleteListener<AuthResult>() {
            @Override
            public void onComplete(@NonNull Task<AuthResult> task) {
                if (!task.isSuccessful()) {
                    Toast.makeText(CreateAccountActivity.this, "Error: Please try again.", Toast.LENGTH_SHORT).show();
                } else {
                    onAuthSuccess(task.getResult().getUser());
                }
                hideProgressDialog();
            }
        });
    }

    private void onAuthSuccess(FirebaseUser user) {
        String username = mUsernameField.getText().toString();

        writeNewUser(user.getUid(), username, user.getEmail());
    }

    private void writeNewUser(String userId, String name, String email) {
        User user = new User(name, email);

        mDatabase.child("users").child(userId).setValue(user);
    }

    @Override
    public void onStart() {
        super.onStart();
        mAuth.addAuthStateListener(mAuthListener);
    }

    @Override
    public void onStop() {
        super.onStop();
        if (mAuthListener != null) {
            mAuth.removeAuthStateListener(mAuthListener);
        }
    }

    private boolean validateForm() {
        boolean valid = true;

        String username = mUsernameField.getText().toString();
        if (TextUtils.isEmpty(username)) {
            mUsernameField.setError("Required");
            valid = false;
        } else {
            mUsernameField.setError(null);
        }

        String email = mEmailField.getText().toString();
        if (TextUtils.isEmpty(email)) {
            mEmailField.setError("Required");
            valid = false;
        } else {
            mEmailField.setError(null);
        }

        String password = mPasswordField.getText().toString();
        if (TextUtils.isEmpty(password)) {
            mPasswordField.setError("Required");
            valid = false;
        } else {
            mPasswordField.setError(null);
        }

        return valid;
    }

}

LoginActivity.java:

public class LoginActivity extends BaseActivity {

    private EditText mEmailField, mPasswordField;

    private FirebaseAuth mAuth;

    private FirebaseAuth.AuthStateListener mAuthListener;

    ProgressDialog mProgressDialog;

    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_login);

        mEmailField = (EditText) findViewById(R.id.loginEmailInput);
        mPasswordField = (EditText) findViewById(R.id.loginPasswordInput);

        Button mLoginButton = (Button) findViewById(R.id.loginButton);

        TextView mCreateAccountLink = (TextView) findViewById(R.id.createAccountLink);

        mProgressDialog = new ProgressDialog(this);
        mProgressDialog.setCancelable(false);
        mProgressDialog.setMessage("Please wait...");

        mAuth = FirebaseAuth.getInstance();

        mAuthListener = new FirebaseAuth.AuthStateListener() {
            @Override
            public void onAuthStateChanged(@NonNull FirebaseAuth firebaseAuth) {
                FirebaseUser user = firebaseAuth.getCurrentUser();
                if (user != null) {
                    startActivity(new Intent(LoginActivity.this, MainActivity.class));
                    finish();
                }
            }
        };

        assert mLoginButton != null;
        mLoginButton.setOnClickListener(new View.OnClickListener() {
            @Override
            public void onClick(View v) {
                signIn(mEmailField.getText().toString(), mPasswordField.getText().toString());
            }
        });

        assert mCreateAccountLink != null;
        mCreateAccountLink.setOnClickListener(new View.OnClickListener() {
            @Override
            public void onClick(View v) {
                startActivity(new Intent(LoginActivity.this, CreateAccountActivity.class));
                finish();
            }
        });
    }

    @Override
    public void onStart() {
        super.onStart();
        mAuth.addAuthStateListener(mAuthListener);
    }

    @Override
    public void onStop() {
        super.onStop();
        if (mAuthListener != null) {
            mAuth.removeAuthStateListener(mAuthListener);
        }
    }

    private void signIn(String email, String password) {
        if (!validateForm()) {
            return;
        }

        showProgressDialog();

        mAuth.signInWithEmailAndPassword(email, password).addOnCompleteListener(this, new OnCompleteListener<AuthResult>() {
            @Override
            public void onComplete(@NonNull Task<AuthResult> task) {
                if (!task.isSuccessful()) {
                    Toast.makeText(LoginActivity.this, "Error: Please try again.", Toast.LENGTH_SHORT).show();
                }
                hideProgressDialog();
            }
        });
    }

    private boolean validateForm() {
        boolean valid = true;

        String email = mEmailField.getText().toString();
        if (TextUtils.isEmpty(email)) {
            mEmailField.setError("Required");
            valid = false;
        } else {
            mEmailField.setError(null);
        }

        String password = mPasswordField.getText().toString();
        if (TextUtils.isEmpty(password)) {
            mPasswordField.setError("Required");
            valid = false;
        } else {
            mPasswordField.setError(null);
        }
        return valid;
    }

}

Answer 1

Field visibility and encapsulation

I think that fields of User can be also private.

Instead of:

public String username;
public String email;

You can use:

private String username;
private String email;

I think that having default constructor for User is not good idea, because somebody can create user without username or email? I don't think so.

The default constructor is the no-argument constructor automatically generated unless you define another constructor

I advise you to remove:

public User() {
}

It allows to avoid future possible errors with user's fields.

DRY(do not repeat yourself)

In BaseActivity you have:

if (mProgressDialog == null) {
  mProgressDialog = new ProgressDialog(this);
  mProgressDialog.setMessage(getString(R.string.loading));
  mProgressDialog.setIndeterminate(true);
  mProgressDialog.setCancelable(false);
}

In CreateAccountActivity you have:

mProgressDialog = new ProgressDialog(this);
mProgressDialog.setCancelable(false);
mProgressDialog.setMessage("Please wait...");

In LoginActivity you have:

mProgressDialog = new ProgressDialog(this);
mProgressDialog.setCancelable(false);
mProgressDialog.setMessage("Please wait...");

Instead of all these lines you can just add some method to BaseActivity for creation progress dialog:

public ProgressDialog createDialog(context Context, isCancelable boolean, String message) 
{
  ProgressDialog mProgressDialog = new ProgressDialog(context);
  mProgressDialog.setMessage(message);
  mProgressDialog.setCancelable(isCancelable);

  return mProgressDialog;
}

Objects of other classes also can use this method because they will inherit it.

Long Method

You have method:

private boolean validateForm() {}

I suggest you to modify name for this method, as example

private boolean isFormValid() {}

Also this method (isFormValid() - I'll use my own name for it) has 'specific smell'. Let's do some refactoring.

Begin with logic of the method:

private boolean isFormValid() { 
  // part 1 - check username

  // part 2 - check email

  // part 3 - check password
}

Now you see that your method has three separate methods, we need to extract them:

private boolean isFormValid() { 
  // part 1 - check username
  boolean isUserNameValid = checkUserName();

  // part 2 - check email
  boolean isEmailValid = checkEmail();

  // part 3 - check password
  boolean isPasswordValid = checkPassword();

  return isUserNameValid && isEmailValid && isPasswordValid;
}

private boolean checkUserName() {
  String username = mUsernameField.getText().toString();

  if (TextUtils.isEmpty(username)) {
    mUsernameField.setError("Required");
    valid = false;
  } else {
    mUsernameField.setError(null);
  }
}

private boolean checkEmail() {
  String email = mEmailField.getText().toString();

  if (TextUtils.isEmpty(email)) {
    mEmailField.setError("Required");

    valid = false;
  } else {
    mEmailField.setError(null);
  }  
}

private boolean checkPassword {
  String password = mPasswordField.getText().toString();

  if (TextUtils.isEmpty(password)) {
    mPasswordField.setError("Required");

    valid = false;
  } else {
    mPasswordField.setError(null);
  }
}

I think that it also seem's not good, we have three method that have the same logic, need to extract similar logic parts into another method:

//you can use another method name
private void checkEditText(EditText et) {
  String field = et.getText().toString();

  if (TextUtils.isEmpty(field)) {
    et.setError("Required");
    return false;
  } else {
    et.setError(null);
    return true;
  }
}

An as a result we have very clean an understandable code for validation the form:

private boolean checkUserName() {
  return checkEditText(mUsernameField);
}

private boolean checkEmail() {
  return checkEditText(mEmailField); 
}

private boolean checkPassword {
  return checkEditText(mPasswordField);
}