Designing a Variable Set

Question

In my application, I need to allow the user to store variables. Variables can only be of specific types, but I effectively handle all variables the same way no matter the type. I would also like to avoid any casting. Therefore, I have several HashMaps for each variable type, which is causing lots of duplicated code. Is there a way to simplify my class?

public class VariableSet {

    private final Map<String, PlayerVariable> players;
    private final Map<String, NumberVariable> numbers;
    private final Set<String> notRemovable;
    private final Set<String> usedVariableNames;

    public VariableSet(){
        players = new HashMap<String, PlayerVariable>();
        numbers = new HashMap<String, NumberVariable>();
        notRemovable = new HashSet<String>();
        usedVariableNames = new HashSet<String>();
        addVariable("Current Player", new PlayerVariable(), players, false);
    }

    public PlayerVariable addPlayer(String variableName){
        PlayerVariable var = new PlayerVariable();
        addVariable(variableName, var, players, true);
        return var;
    }

    public NumberVariable addNumber(String variableName){
        NumberVariable var = new NumberVariable();
        addVariable(variableName, var, numbers, true);
        return var;
    }

    private <T extends Variable<?>> void addVariable(String variableName, T variable, Map<String, T> map, boolean removable){
        if (usedVariableNames.contains(variableName)){
            throw new VariableExistsException();
        }
        usedVariableNames.add(variableName);
        map.put(variableName, variable);
        if (!removable){
            notRemovable.add(variableName);
        }
    }

    public void removeNumber(String variableName){
        removeVariable(variableName, numbers);
    }

    public void removePlayer(String variableName){
        removeVariable(variableName, players);
    }

    private <T extends Variable<?>> void removeVariable(String variableName, Map<String, T> map){
        if (notRemovable.contains(variableName)){
            throw new VariableNotRemovableException();
        }
        usedVariableNames.remove(variableName);
        map.remove(variableName);
    }

    public boolean hasPlayerVariable(String variableName){
        return players.containsKey(variableName);
    }

    public boolean hasNumberVariable(String variableName){
        return numbers.containsKey(variableName);
    }

    public Set<String> getVariableNames(){
        return usedVariableNames;
    }

    public void setPlayer(String variableName, PlayerVariable player){
        players.put(variableName, player);
    }


    public void setNumber(String variableName, NumberVariable number){
        numbers.put(variableName, number);
    }
}

All variables extend the same abstract class, so using generics is probably the preferred method, but I can't figure out a way to create a data structure that automatically picks the correct variable type without casting.

Answer 1

The only way to do what you are trying to do, and remove the duplication, is really to cast.

What I would do is to use this kind of map:

private final Map<Class<? extends Variable>, Map<String, Variable>> variables;

To add something to the map, you could do:

variables.get(NumberVariable.class).put("key", value);

And to get:

return (NumberVariable) variables.get(NumberVariable.class).get("key");

You could even write a generic method:

private <T> T getVariable(Class<T> clazz, String key) {
    return clazz.cast(variables.get(clazz).get(key));
}

Note that this shouldn't even give any compiler warnings.

---

Leaking inner variable

Considering your getVariableNames method, imagine if I would call your code like this:

myVariableSet.getVariableNames.clear();

Whoops! I broke it!

The simple fix is this:

public Set<String> getVariableNames() {
    return new HashSet<>(usedVariableNames);
}

That is, return a copy of the data, then I can manipulate it all I want - I won't break anything!

I wouldn't use a usedVariableNames at all though and just do a map.containsKey(key) directly. If you want to make sure that all variable names are unique though, even if they are of different classes, then go ahead and keep this variable.