Susannah Potts Susannah Potts - 4 months ago 14
Java Question

How should I break this block into smaller functional units?

I'm writing an application to help my friends run some numbers to balance a game.

This has become a larger project than I initially thought it would be, due to the large number of classes and stat variables.

I'm in the middle of making

HeroObject
s which are supposed to represent the stats of the hero. This object is extended by each class (i.e.
Marksman
) which provides specific functionality related to the class. I'm reading these heros in via a
csv
file so multiple classes and skill formulas can be tested at once. Here's the function I'm using to make the
HeroObject
s:

private HeroObject makeHero(String[] heroLine) {
switch(heroLine[11].toLowerCase()) {
case "rogue":
return new Rogue(Integer.valueOf(heroLine[0]), Integer.valueOf(heroLine[1]),
Integer.valueOf(heroLine[2]), Integer.valueOf(heroLine[3]),
Integer.valueOf(heroLine[4]), Integer.valueOf(heroLine[5]),
Integer.valueOf(heroLine[6]), Integer.valueOf(heroLine[7]),
Integer.valueOf(heroLine[8]), Integer.valueOf(heroLine[9]),
heroLine[10]);
case "marksman":
return new Marksman(Integer.valueOf(heroLine[0]), Integer.valueOf(heroLine[1]),
Integer.valueOf(heroLine[2]), Integer.valueOf(heroLine[3]),
Integer.valueOf(heroLine[4]), Integer.valueOf(heroLine[5]),
Integer.valueOf(heroLine[6]), Integer.valueOf(heroLine[7]),
Integer.valueOf(heroLine[8]), Integer.valueOf(heroLine[9]),
heroLine[10]);
case "knight":
return new Knight(Integer.valueOf(heroLine[0]), Integer.valueOf(heroLine[1]),
Integer.valueOf(heroLine[2]), Integer.valueOf(heroLine[3]),
Integer.valueOf(heroLine[4]), Integer.valueOf(heroLine[5]),
Integer.valueOf(heroLine[6]), Integer.valueOf(heroLine[7]),
Integer.valueOf(heroLine[8]), Integer.valueOf(heroLine[9]),
heroLine[10]);
case "lancer":
return new Lancer(Integer.valueOf(heroLine[0]), Integer.valueOf(heroLine[1]),
Integer.valueOf(heroLine[2]), Integer.valueOf(heroLine[3]),
Integer.valueOf(heroLine[4]), Integer.valueOf(heroLine[5]),
Integer.valueOf(heroLine[6]), Integer.valueOf(heroLine[7]),
Integer.valueOf(heroLine[8]), Integer.valueOf(heroLine[9]),
heroLine[10]);
case "windadept":
return new WindAdept(Integer.valueOf(heroLine[0]), Integer.valueOf(heroLine[1]),
Integer.valueOf(heroLine[2]), Integer.valueOf(heroLine[3]),
Integer.valueOf(heroLine[4]), Integer.valueOf(heroLine[5]),
Integer.valueOf(heroLine[6]), Integer.valueOf(heroLine[7]),
Integer.valueOf(heroLine[8]), Integer.valueOf(heroLine[9]),
heroLine[10]);
case "wateradept":
return new WaterAdept(Integer.valueOf(heroLine[0]), Integer.valueOf(heroLine[1]),
Integer.valueOf(heroLine[2]), Integer.valueOf(heroLine[3]),
Integer.valueOf(heroLine[4]), Integer.valueOf(heroLine[5]),
Integer.valueOf(heroLine[6]), Integer.valueOf(heroLine[7]),
Integer.valueOf(heroLine[8]), Integer.valueOf(heroLine[9]),
heroLine[10]);
case "lightningadept":
return new LightningAdept(Integer.valueOf(heroLine[0]), Integer.valueOf(heroLine[1]),
Integer.valueOf(heroLine[2]), Integer.valueOf(heroLine[3]),
Integer.valueOf(heroLine[4]), Integer.valueOf(heroLine[5]),
Integer.valueOf(heroLine[6]), Integer.valueOf(heroLine[7]),
Integer.valueOf(heroLine[8]), Integer.valueOf(heroLine[9]),
heroLine[10]);
case "lightadept":
return new LightAdept(Integer.valueOf(heroLine[0]), Integer.valueOf(heroLine[1]),
Integer.valueOf(heroLine[2]), Integer.valueOf(heroLine[3]),
Integer.valueOf(heroLine[4]), Integer.valueOf(heroLine[5]),
Integer.valueOf(heroLine[6]), Integer.valueOf(heroLine[7]),
Integer.valueOf(heroLine[8]), Integer.valueOf(heroLine[9]),
heroLine[10]);
case "iceadept":
return new IceAdept(Integer.valueOf(heroLine[0]), Integer.valueOf(heroLine[1]),
Integer.valueOf(heroLine[2]), Integer.valueOf(heroLine[3]),
Integer.valueOf(heroLine[4]), Integer.valueOf(heroLine[5]),
Integer.valueOf(heroLine[6]), Integer.valueOf(heroLine[7]),
Integer.valueOf(heroLine[8]), Integer.valueOf(heroLine[9]),
heroLine[10]);
case "fireadept":
return new FireAdept(Integer.valueOf(heroLine[0]), Integer.valueOf(heroLine[1]),
Integer.valueOf(heroLine[2]), Integer.valueOf(heroLine[3]),
Integer.valueOf(heroLine[4]), Integer.valueOf(heroLine[5]),
Integer.valueOf(heroLine[6]), Integer.valueOf(heroLine[7]),
Integer.valueOf(heroLine[8]), Integer.valueOf(heroLine[9]),
heroLine[10]);
case "earthadept":
return new EarthAdept(Integer.valueOf(heroLine[0]), Integer.valueOf(heroLine[1]),
Integer.valueOf(heroLine[2]), Integer.valueOf(heroLine[3]),
Integer.valueOf(heroLine[4]), Integer.valueOf(heroLine[5]),
Integer.valueOf(heroLine[6]), Integer.valueOf(heroLine[7]),
Integer.valueOf(heroLine[8]), Integer.valueOf(heroLine[9]),
heroLine[10]);
case "darkadept":
return new DarkAdept(Integer.valueOf(heroLine[0]), Integer.valueOf(heroLine[1]),
Integer.valueOf(heroLine[2]), Integer.valueOf(heroLine[3]),
Integer.valueOf(heroLine[4]), Integer.valueOf(heroLine[5]),
Integer.valueOf(heroLine[6]), Integer.valueOf(heroLine[7]),
Integer.valueOf(heroLine[8]), Integer.valueOf(heroLine[9]),
heroLine[10]);
}

return null;
}


I don't have to point out how much of this is repetitive code, but I'm unsure how I can break the case section into its own class without checking what the class is again.

tl;dr:
Each class is extending
HeroObject
and thus has the same constructor but I still need these to be created via their respective interfaces. How can I functionally separate the following blocks:

return new SomeClass(Integer.valueOf(heroLine[0]), Integer.valueOf(heroLine[1]),
Integer.valueOf(heroLine[2]), Integer.valueOf(heroLine[3]),
Integer.valueOf(heroLine[4]), Integer.valueOf(heroLine[5]),
Integer.valueOf(heroLine[6]), Integer.valueOf(heroLine[7]),
Integer.valueOf(heroLine[8]), Integer.valueOf(heroLine[9]),
heroLine[10]);

Answer

You should certainly separate reading the Excel file and creating instances from the Hero class.

I'd create a HeroFactory (aka virtual constructor) that could give me instances of Hero and any other subclass.

It looks like the columns in the Excel file are the same for each case. Instead of a switch statement, pass in the type of object you want.

public class HeroFactory {

    public static final Hero createHero(Class heroClass) {
      // create the type of Hero here based on Class passed in.  You can use reflection to make it clean.  No switch needed.
    }
}

It'd be easy to cut down on the amount of code with another constructor:

public class Hero {
    public Hero(String [] parameters) {
        // initialize your stuff by iterating over the parameters  instead of passing 11 values.
    }
}