Java (Android) abstract class correct implementation

Question

I am looking for some comments on my code and whether or not this is the best way to create an abstract class. In addition, some of the abstract methods are rarely called so I use the base variables within the subclass to make sure onCreateDialog selects the correct stuff. Hence, the getTitle() > 0 conditional.

public abstract class BaseDialogFragment extends DialogFragment {

// Base Variables
public static final int NONE = -1;
public static final int ITEM_SET = 1;

// Extras
public String extraTitle = "";
public String extraMessage = "";
private View dialogView;
private ListView list;

public BaseDialogFragment() {}

@Override
public Dialog onCreateDialog(Bundle savedInstanceState) {
    ContextThemeWrapper context = null;
    String theme = ActivityPrefHandler.getTheme(getActivity());
    if (theme.equals(PreferencesActivity.MATERIAL)) {
        context = new ContextThemeWrapper(getActivity(), R.style.ThemeMainDialog);
    } else if (theme.equals(PreferencesActivity.LIGHT)) {
        context = new ContextThemeWrapper(getActivity(), R.style.ThemeLightDialog);
    } else if (theme.equals(PreferencesActivity.DARK)) {
        context = new ContextThemeWrapper(getActivity(), R.style.ThemeDarkDialog);
    }
    CustomAlertDialogBuilder builder = new CustomAlertDialogBuilder(context);
    if (getIcon() > 0) {
        builder.setIcon(getIcon());
    }
    if (getTitle() > 0) {
        if (getExtraTitle().equals("")) {
            builder.setTitle(getTitle());
        } else {
            builder.setTitle(getExtraTitle());
        }
    }
    if (getMessage() > 0) {
        if (getExtraMessage().equals("")) {
            builder.setMessage(getMessage());
        } else {
            builder.setMessage(getExtraMessage());
        }
    }
    if (getDialogView() > 0) {
        LayoutInflater inflater = getActivity().getLayoutInflater();
        dialogView = inflater.inflate(getDialogView(), null);
        builder.setView(dialogView);
    }
    if (getPositiveButtonText() > 0) {
        builder.setPositiveButton(getPositiveButtonText(), new OnClickListener() {
            @Override
            public void onClick(DialogInterface dialog, int which) {
                onConfirmClick();
            }
        });
    }
    if (getNeutralButtonText() > 0) {
        builder.setNeutralButton(getNeutralButtonText(), new OnClickListener() {
            @Override
            public void onClick(DialogInterface dialog, int which) {
                onNeutralClick();
            }
        });
    }
    builder.setNegativeButton(R.string.cancel, new OnClickListener() {
        @Override
        public void onClick(DialogInterface dialog, int which) {
            onCancelClick();
        }
    });
    return builder.create();
}

@Override
public void onDestroyView() {
    extraTitle = "";
    super.onDestroyView();
}

public abstract void setListener(final OnDialogConfirmed l);

public abstract int getIcon();

public abstract int getTitle();

public abstract int getMessage();

public abstract int getDialogView();

public abstract int getPositiveButtonText();

public abstract int getNeutralButtonText();

public abstract void onConfirmClick();

public abstract void onCancelClick();

public abstract void onNeutralClick();

public void setTitle(String title) {
    extraTitle = title;
}

public String getExtraTitle() {
    if (!extraTitle.equals("")) {
        return extraTitle;
    } else {
        return "";
    }
}

public void setExtraMessage(String message) {
    extraMessage = message;
}

public String getExtraMessage() {
    if (!extraMessage.equals("")) {
        return extraMessage;
    } else {
        return "";
    }
}

public void setListViewAdapter(SimpleCursorAdapter adapter) {
    if (dialogView != null) {
        if (dialogView.findViewById(R.id.fragment_listview) != null) {
            list = (ListView) dialogView.findViewById(R.id.fragment_listview);
            if (adapter != null) {
                list.setAdapter(adapter);
            }
        }
    }
}

public ListView getListView() {
    return list;
}

// generic methods omitted

}

Answer 1

Here are my thoughts going line by line:

• Comments like // Base Variables are useless, so get rid of them. The first two variables you declare are in fact constants and their role is obvious from their instantiation. It looks like you just have two performing a related role, though. Maybe an enum would be more appropriate?

• No need for the empty constructor.

• The initial theme if-else block rubs me the wrong way, though I might just be pedantic, since it's so terse. Still, what I would rather see is a mapping of PreferenceActivity enum values to R.style values so you can do a simple lookup, remove the duplication, and add error handling if a key isn't found.

• onCreateDialog does a lot of different things. You can extract methods to raise the abstraction level, since all you seem to need is you CustomAlertDialogBuilder object passed in.

• If you are able to use Java 8, you can greatly simply all of your OnClickListener methods.

• What are getExtraTitle and getExtraMessage even doing? Look at getExtraTitle. It says, if extraTitle isn't the empty string, return extraTitle, otherwise return the empty string. That's equivalent to just return extraTitle;.

  I have used this format myself, however, but with the consideration of the fact that the string might be null. In that case, I would use the Apache commons-lang lib to do the following:

  public String getExtraTitle() {
    return Strings.isNullOrEmpty(extraTitle) ? "" : extraTitle;
  }