Gideon Sassoon
Gideon Sassoon

Reputation: 604

A more elegant way of setting up an object based on other values

First apologies on the state of this Title, I do not know what the question I am actually asking is, meaning I don't know the correct terminology to use so if someone could tell me that I'd be happy to change the title or delete and repost this.

So currently in a project I am doing I am having to set up an object of values, ignore the fact that it's a realmObject it won't matter.

public class Skills extends RealmObject {

    private boolean acrobaticsMarked = false;
    private int acrobaticsValue;
    private final String acrobaticsAbility = "DEX";

    private boolean animalHealingMarked = false;
    private int animalHealingValue;
    private final String animalHealingAbility = "WIS";

    private boolean arcanaMarked = false;
    private int arcanaValue;
    private final String arcanaAbility = "INT";

    private boolean athleticsMarked = false;
    private int athleticsValue;
    private final String athleticsAbility = "STR";

    private boolean deceptionMarked = false;
    private int deceptionValue;
    private final String deceptionAbility = "CHA";

    private boolean historyMarked = false;
    private int historyValue;
    private final String historyAbility = "INT";

    private boolean insightMaarked = false;
    private int insightValue;
    private final String insightAbility = "WIS";

    private boolean intimidationMarked = false;
    private int intimidationValue;
    private final String intimidationAbility = "CHA";

    private boolean investigationMarked = false;
    private int investigationValue;
    private final String investigationAbility = "INT";

    private boolean medicineMarked = false;
    private int medicineValue;
    private final String medicineAbility = "WIS";

    private boolean natureMarked = false;
    private int natureValue;
    private final String natureAbility = "INT";

    private boolean perceptionMarked = false;
    private int perceptionValue;
    private final String perceptionAbility = "WIS";

    private boolean performanceMarked = false;
    private int performanceValue;
    private final String performanceAbility = "CHA";

    private boolean persuasionMarked = false;
    private int persuasionValue;
    private final String persuasionAbility = "CHA";

    private boolean religionMarked = false;
    private int religionValue;
    private final String religionAbility = "INT";

    private boolean sleightOfHandMarked = false;
    private int sleightOfHandValue;
    private final String sleightOfHandAbility = "DEX";

    private boolean stealthMarked = false;
    private int stealthValue;
    private final String stealthAbility = "DEX";

    private boolean survivalMarked = false;
    private int survivalValue;
    private final String survivalAbility = "WIS";

As you can see each of these have an ability value STR, DEX, CON, INT, WIS, CHA. Yes it's D&D. So these values have been set up in another class. I'm not going to paste the whole class you can go find it here but it has the following values which will already be filled out at this point.

int strength;
int dexterity;
int constitution;
int intelligence;
int wisdom;
int charisma;

Currently I am setting the values like this

public void setValues(Abilities abilities) {
    this.setAcrobaticsValue(abilities.getDexterity());
    this.setAnimalHealingValue(abilities.getWisdom());
    this.setArcanaValue(abilities.getIntelligence());
    this.setAthleticsValue(abilities.getStrength());
    this.setDeceptionValue(abilities.getCharisma());
    this.setHistoryValue(abilities.getIntelligence());
    this.setInsightValue(abilities.getWisdom());
    this.setIntimidationValue(abilities.getCharisma());
    this.setInvestigationValue(abilities.getIntelligence());
    this.setMedicineValue(abilities.getWisdom());
    this.setNatureValue(abilities.getIntelligence());
    this.setPerceptionValue(abilities.getWisdom());
    this.setPerformanceValue(abilities.getCharisma());
    this.setPersuasionValue(abilities.getCharisma());
    this.setReligionValue(abilities.getIntelligence());
    this.setSleightOfHandValue(abilities.getDexterity());
    this.setStealthValue(abilities.getDexterity());
    this.setSurvivalValue(abilities.getWisdom());
}

I want to know if there's a more elegant way to run this class line by line so it first goes through each of the classes int values and on each one read each of the String's Ability values in the Skills class and set the skills Value by retrieving the stated String from Abilities. eg. Upon reading that acrobaticsAbility is equal to DEX it goes and retrieves the Dexterity value.

Upvotes: 0

Views: 90

Answers (4)

c0der
c0der

Reputation: 18812

As rough structure consider the following :

    public class Abilities{

        enum Ability {DEXTERITY, WISDOM; }//add more

        private Map<Ability, Integer> abilities;

        public Abilities(){
            abilities = new HashMap<>();
        }

        public int getValue(Ability ability) {
            return abilities.get(ability);
        }

        public void setValue(Ability ability, int value) {
            abilities.put(ability, value);
        }
    } 

And:

public class Skills {

    enum SkillName{DEX, WIS, INT, STR, CHA;}

    private Skill acrobatics;
    private Skill animalHealing;
    private Abilities abilities;

    Skills(){
        abilities = new Abilities();
        abilities.setValue(Ability.DEXTERITY, 5);
        abilities.setValue(Ability.WISDOM, 3);
        acrobatics    = new Skill(SkillName.DEX, Ability.DEXTERITY, false);
        animalHealing = new Skill(SkillName.WIS, Ability.WISDOM, false);
    }

    public boolean isFlag(Skill skill) {
        return skill.isFlag();
    }

    public void setFlag(Skill skill, boolean flag) {
        skill.setFlag(flag);
    }

    public int getValue(Skill skill) {
        return skill.getValue();
    }

    public void setValue(Skill skill, int value) {
        skill.setValue(value);
    }

    class Skill{

        private SkillName name;
        private Ability ability;
        boolean flag;

        Skill(SkillName name, Ability ability, Boolean flag){

            this.name = name;
            this.ability = ability;
            this.flag = flag;
        };

        public boolean isFlag() {return flag;}
        public void setFlag(boolean flag) { this.flag = flag;}
        public int getValue() { return abilities.getValue(ability);}
        public void setValue(int value) {abilities.setValue(ability,value);}
        public String getName() {return name.toString();}
        public void setName(SkillName name) {this.name = name;  }
    }
}

Upvotes: 0

Steve11235
Steve11235

Reputation: 2923

I don't have a solution, so this may not be helpful; rather, I have some suggestions.

Skills seems to have several "skill" entries. You could create a Skill class with the three attributes encapsulated there.

You would then have Skills with several Skill entries. Instead, you could have a Map of Skill by "ability", which could either be a String constant (static final String) or an enum.

At this point, you might be able eliminate the Skill class and have a Map of Integer by "ability". The "marked" property could be replaced by noting whether the key is found in the map. However, keeping the Skill class allows you to add other properties to it.

As far as simplifying setValues(), that would involve refactoring Abilities along the same lines, so that you could loop through each Ability and add a Skill.

Upvotes: 0

Norrey Okumu
Norrey Okumu

Reputation: 21

My best bet would be to create a relationship between the Skills and the Ability classes.

public class Skills extends RealmObject {
   // Remove all fields that can be obtained from ability
   private Ability ability;

   public int getAcrobaticsValue(){
   return ability.getWisdom();
 }
}

Upvotes: 0

Jeroen van Dijk-Jun
Jeroen van Dijk-Jun

Reputation: 1038

You should create a class Ability with 3 properties: marked, value and (static) ability. This object will replace every set of 3 properties. Then you can change setValues to handle a collection of Ability objects. Maybe you only need a normal setter and keep the collection as it is. Or you can use a map, where you have the string ability as the key. In this way you can locate the needed ability with that key.

You can also have the ability keys put into an enum, e.g. AbilityType, this improves readability and mitigate possible typo's. Then the static string will be a static AbilityType, and so will be the type of the key in your map.

Upvotes: 1

Related Questions