1

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)
|improve this question
  • What is an 'IV'? – Kilian Foth Jul 31 '14 at 12:58
  • Instance variable, edited title when I realised I shouldnt abbreviate it in the title =/ – FinalFind Jul 31 '14 at 12:58
  • 2
    In short, yes it does. – Telastyn Jul 31 '14 at 13:09
  • Any chance of an explanation for that statement? Also are you saying that the class setup is flawed, or that I need to spread the objects around? – FinalFind Jul 31 '14 at 13:13
  • @FinalFind I imgaine Telastyn meant you should use some sort of collection docs.oracle.com/javase/7/docs/api/java/util/Collection.html – jk. Jul 31 '14 at 13:16
3

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)

|improve this answer
  • Well I need duel to be able to access each individual character, each of which has a different weapon, persona and stats. In a previous plan I considered putting characters stats in the character class aswell, but didnt think that sounded right (tho admittedly I have the characters name in there). I suppose I should put characters in something like this? public interface Duelists extends Iterable<Character> – FinalFind Jul 31 '14 at 14:15
  • I guess I dont quite see how I can assign each Character object a predefined weapon, Persona, Character stats etc if all these objects are in seperate classes. – FinalFind Jul 31 '14 at 14:17
  • @FinalFind - Character has a Weapon property you can assign. It has an ActivePersona property you can assign, etc. If they're fixed, then make them readonly and assigned during construction. – Telastyn Jul 31 '14 at 14:26
  • Well the player will be able to choose their weapon and persona etc, the enemy they are against is fixed. Might even like to randomize the enemies ActivePersona and Weapon at some point – FinalFind Jul 31 '14 at 14:31
  • Also when u say bundling into classes seems like the best approach, do you mean create a class for each object that inherits from their respective main class (e.g. a MitsuruStats class that inherits from CharacterStats and a Mitsuru character that inherits from Character) – FinalFind Jul 31 '14 at 15:55

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.