CODEWITHSUNDEEP
javaandroidloopsreflection
Walker
Walker

Reputation: 1225

Getting around similar-variable repetition in if statements

I believe I'm asking a slightly different take on the perennial question of avoiding code repetition. The setup is fairly standard--a bunch of if statements taking similar actions. Below you'll find a short example.

I'm trying to figure out the best way to tackle situations like this, in terms of code efficiency, compactness, and programmer ease. Note that separate solutions addressing each of those points are fine, indeed preferable, as I doubt one solution hits all three.

For example, one possible solution that came to mind seems like it would be ungainly and slow, especially on a mobile device like android, plus it would only work if the variables were instance variables, and not merely local (which may very well be the case). The idea involved a for statement whose initialization expression would use double-brace initialization to fill a hashmap with strings corresponding to the variables involved (e.g. "installText" maps to "installPermission"), then the for-loop would contain one of the if--else-if--else-if clauses below, which would use reflection to access the variables by their names stored in the hashmap.

How can I do this better? Thanks in advance for your time and advice!

if (installText.equals("Default")) {
    installPermission = DEFAULT;
} else if (installText.equals("Allow")) {
    installPermission = ENABLED;
} else if (installText.equals("Disallow")) {
    installPermission = DISABLED;
}
if (uninstallText.equals("Default")) {
    uninstallPermission = DEFAULT;
} else if (uninstallText.equals("Allow")) {
    uninstallPermission = ENABLED;
} else if (uninstallText.equals("Disallow")) {
    uninstallPermission = DISABLED;
}
if (runText.equals("Default")) {
    runPermission = DEFAULT;
} else if (runText.equals("Allow")) {
    runPermission = ENABLED;
} else if (runText.equals("Disallow")) {
    runPermission = DISABLED;
}

Upvotes: 4

Views: 118

Answers (3)

Moataz Elmasry
Moataz Elmasry

Reputation: 2519

if you are 100% sure, that the string value will be one of these three. then you can do it as follows:

Map<String,String> map;
map.put("Default", "Default");
map.put("Allow", "Allow");
map.put("Disallow", "Disallow");
installPermission = map.get(installText);
uninstallPermission = map.get(uninstallText);
runPermission = map.get(runText);

cheers

Upvotes: 1

carlspring
carlspring

Reputation: 32629

Extract the value checking to a separate method (I'm re-using part of Simeon Visser's answer with improvements):

public String getPermission(String permission)
{
    String state = null;

    // If this is Java 7:
    /*
    switch(permission)
    {
        case "Allow":
             state = ENABLED;
             break;
        case "Disallow":
             state = DISABLED;
             break;
        case "Default":
        default:
             state = DEFAULT;
             break;  
    }
    */

    // If this is Java < 7:
    if (permission.equalsIgnoreCase("allow"))
        state = ENABLED;
    else if (permission.equalsIgnoreCase("disallow"))
        state = DISABLED;
    else
        state = DEFAULT;

    return state;
}

public void callingMethod(String permission)
{
     installPermission = getPermissionState(permission);
     uninstallPermission = getPermissionState(permission);
     runPermission = getPermissionState(permission);
}

Furthermore, ENABLED, DISABLED and DEFAULT should simply be defined as constants in your class:

public static final String ENABLED = "ENABLED";
public static final String DISABLED = "DISABLED";
public static final String DEFAULT = "DEFAULT";

However, I still fail to grasp your idea with the reflection...? I don't find it necessary here, unless you have a use case whose code you're not showing at the moment...?

Upvotes: 1

Simeon Visser
Simeon Visser

Reputation: 122436

Are you using Java 7? In that case you can use the switch statement which now supports String values:

switch(installText) {
    case "Allow":
        installPermission = ENABLED;
        break;
    case "Disallow":
        installPermission = DISABLED;
        break;
    case "Default":
        installPermission = DEFAULT;
        break;  
    default:
        installPermission = DEFAULT;
        break;  
}

Upvotes: 2

Related Questions