Logging in to Android application using Firebase

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) f

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);
}

StackExchange Code Review Q#132135, answer score: 2