CODEWITHSUNDEEP
javaandroidenums
Kaloyan Roussev
Kaloyan Roussev

Reputation: 14711

Is this a good case where I should use Java Enum?

In an android asynctask of mine I have the following code:

public class AddNewItemToServerAsync extends AsyncTask<String, String, Bundle> {

    private static final String TAG_SUCCESS = "success";
    private static final String TAG_ITEM_ID = "item_id";
    private static final String TAG_TRANSACTION_ID = "transaction_id";
    private static final String TAG_TIME_ITEM_WAS_ADDED_TO_DB = "time_product_added";
    private static final String TAG_BAD_REQUEST = "Bad request";
    private static final String TAG_DB_PROBLEM = "Database Problem";
    private static final String TAG_UNKNOWN_PROBLEM = "Unknown Problem"; 
    private void setReason(String reason) {
        this.reason = reason;
    }

    @Override
    protected void onPreExecute() {
        ShowLoadingMessage.loading(context, Configurationz.LoadingMessages.ADDING_NEW_ITEM_TO_USER_COLLECTION);
    }

    @Override
    protected Bundle doInBackground(String... args) {
                         //code here
                return dataRegardingTheItemJustAdded;

            } else if (success == Configurationz.HttpResponses.FAILURE_BAD_REQUEST) {
                // report tool
                setReason(TAG_BAD_REQUEST);
                return null;

            } else if (success == Configurationz.HttpResponses.FAILURE_DATABASE_PROBLEM) {
                setReason(TAG_DB_PROBLEM);
                return null;
            } else {
                setReason(TAG_UNKNOWN_PROBLEM);
                return null;
            }
        } catch (JSONException e) {
            e.printStackTrace();
            setReason(TAG_UNKNOWN_PROBLEM);
            return null;
        }

    }

    @Override
    protected void onPostExecute(Bundle dataRegardingTheItemJustAdded) {
        ShowLoadingMessage.dismissDialog();
        if (dataRegardingTheItemJustAdded != null) {
            if (listener != null) {
                listener.onAddNewItemSuccess(dataRegardingTheItemJustAdded);
            }
        } else {
            if (listener != null) {
                if (reason != null) {
                    listener.onAddNewItemFailure(reason);
                }
            }
        }
        super.onPostExecute(dataRegardingTheItemJustAdded);
    }
}

and then the listener class goes like this:

private static final String TAG_BAD_REQUEST = "Bad request";
private static final String TAG_DB_PROBLEM = "Database Problem";
private static final String TAG_UNKNOWN_PROBLEM = "Unknown Problem";

@Override
public void onAddNewItemFailure(String reason) {
    if (reason.contentEquals(TAG_BAD_REQUEST)) {
        //toast 
    } else if (reason.contentEquals(TAG_DB_PROBLEM)) {
        //toast 
    } else if (reason.contentEquals(TAG_UNKNOWN_PROBLEM)) {
        //toast 
    }

}

I think this is not well optimized in terms of the tags being defined twice. Should I use ENUM here and if the answer is yes, how?

Or should I move these tags to the congfigurationz class and make them just classic static variables>

Upvotes: 1

Views: 144

Answers (3)

asteri
asteri

Reputation: 11572

An enum doesn't make sense for your constants because they aren't logically connected.

private static final String TAG_SUCCESS = "success";
private static final String TAG_ITEM_ID = "item_id";
private static final String TAG_TRANSACTION_ID = "transaction_id";
private static final String TAG_TIME_ITEM_WAS_ADDED_TO_DB = "time_product_added";
private static final String TAG_BAD_REQUEST = "Bad request";
private static final String TAG_DB_PROBLEM = "Database Problem";
private static final String TAG_UNKNOWN_PROBLEM = "Unknown Problem";

You actually have two groups of things here. You have what I assume are column names and then some status Strings. An enum, just like a class, should be a logically self-contained and consistent unit. You could of course define two enums to capture both sets, if you wanted.

public enum TagStatus {
    SUCCESS ("Success"),
    BAD_REQUEST ("Bad Request"),
    DATABASE_PROBLEM ("Database Problem"),
    UNKNOWN_PROBLEM ("Unknown Problem");

    private final String description;

    private TagStatus(String description) {
        this.description = description;
    }

    public String getDescription() {
        return description;
    }

    @Override
    public String toString() {
        return description;
    }
}

... and then ...

public enum TagColumn {
    ITEM_ID ("item_id"),
    // ... and so on

This is a lot of framework to add on, and it's really a subjective design consideration. In many cases it makes more sense simply to expose the constants as public for the rest of your API to use.

Upvotes: 3

Danish
Danish

Reputation: 3798

I my opinion, this is the perfect example of using Java Enums. Not will this only consolidate your tags, it will provide type safety.

You can define your enum as

public enum Tag {
    SUCCESS, ITEM_ID, TRANSACTION_ID // Plus all the other tags
}

and use them as 

@Override
public void onAddNewItemFailure(Tag reason) {
    if (Tag.BAD_REQUEST.equals(reason)) {
        //toast 
    } else if (Tag.DB_PROBLEM.equals(reason)) {
        //toast 
    } else if (Tag.UNKNOWN_PROBLEM.equals(reason)) {
        //toast 
    }

}

Upvotes: 3

user1445967
user1445967

Reputation: 1570

You "could" solve this problem with an Emum but since you don't need any of the additional functionality that Enum provides, I would argue it is not worth the added complexity.

You can make the tags public and access them from another class. Even better, define a Tags class and put all the tags in there. Then inside your AddNewItemToServerAsync class you can just refer to a tag in this way: Tags.BAD_REQUEST.

And this way, your tags are only defined in one place. Better yet, inside a class name that makes it very obvious where to look for existing tags and where any new tags should be added.

Upvotes: 1

Related Questions