Does lots of Instance Variables within my class suggest flawed Design?

Question

Ok so I am setting out to consolidate some learning I have been doing. What I am going to do is create a small game relating to a game I enjoy called Persona.

For reference a persona you can think of as a pokemon.

Basically I want the flow to be user chooses a character, a weapon, and a persona (from a list of predefined personas and people i.e. predefined stats and spells for the persona and predefined weapons, stats and personas for the characters). They will choose and I will use maths/ifs/whiles etc to calculate winners.

I currently have a Characters class (with Persona, Stats and Weapon instance variables), a Character Stats class (health, mana etc IVs), a Persona class(with persona stats and name iv), a Persona stats class (agility strength etc IVs) (later I will make the PersonaSpells class) and a Weapons class (attack, hit etc IVs), as well as a Duel class (where all the calculations will be run and methods for player select, enemy selection etc. Additionally with my current implementation all character objects, persona objects and weapon objects etc will be defined in this class and assigned using for example using Character char = new Character(Name, Health, Persona, Weapon, ...), with constructors for persona and weapon defined before this. Finally a main class where I will effectively use an object of the duel class to run the game, and keep the main class and method clearer.

My question is, is this design flawed in that, I end up having to declare loads of objects (as IVs) from constructors of my classes. (So I have an object of all 20 weapons, all 10 characters, all 20 Personas, all 10 character stats, all 20 persona stats) All within my Duel class. Code example below:

private Weapon deusXiphos = new Weapon();
(x 20 more like this)

private PersonaStats artemisiaStats = new PersonaStats();
(x 20 more like this)

private Persona artemisia = new Persona();
(* 20 more like this)

private CharacterStats mitsuruStats = new CharacterStats(400, 250);
(* 10 more like this)

private Character mitsuru = new Character("Mitsuru", artemisia, deusXiphos, mitsuruStats);
(* 10 more like this)

Answer 1

Yes, having many instance variables is a sign that you have a flawed design.

Personally, I put the line around 3 variables if they're mutable and 5 if they're not (if the class is anything more than plain old data). This is uncommonly low. But no matter where the line is, there is a line and having 80 instance variables in a class is going to be over pretty much everyone's line.

Why?

• It likely means you've violated the Single Responsibility Principle. When your class has many instance variables, it's unlikely that they provide a cohesive single responsibility for that class. When that fails, it becomes difficult to change the class for one reason without breaking others. It makes it hard to test the functionality of the class in isolation since you need all of its stuff to be there.
• It likely means you've violated the Don't Repeat Yourself guideline. What happens when you want to change a field on those stats? Now you have to go to the 30 copies of them and change them all. This increases the chance that you make a mistake and miss one.
• It makes it hard to use the Dependency Inversion Principle, because it will be super annoying to pass in 80 objects into your constructor.

Bundling the variables into classes seems like the best approach. Why does the Duel need Mitsuru and Mitsuru's stats? Shouldn't Mitsuru have her own stats?

(though by the letter of the law, that would then lead to a violation of the Law of Demeter when Duel calls mitsuru.Stats.Strength. I don't think that's a particularly egregious error, since it's unlikely that this coupling would cause future errors, and it's far better than the alternative)