Creating enchantment objects based on configuration

Question

I'm searching a way to literally shorten my code. For the moment my code is this;

    if(getDashConfig().getBoolean("enchantments.multifirearrow.enabled")){
        MultiFireArrow fire = new MultiFireArrow();
        DashEnchant.getEnchants().put(1, fire);
        DashEnchant.getStringEnchants().put("multifirearrow", fire);
        if(debug()){
            getLogger().info("MultiFireArrow enchant registred");
        }
    if(getDashConfig().getBoolean("enchantments.bolt.enabled")){    
        Bolt bolt = new Bolt();
        DashEnchant.getEnchants().put(2, bolt);
        DashEnchant.getStringEnchants().put("bolt", bolt);
        if(debug()){ getLogger().info("Bolt enchant registred"); }
        }
    if(getDashConfig().getBoolean("enchantments.randomspeed.enabled")){
        SpeedRandom speed = new SpeedRandom();
        DashEnchant.getEnchants().put(3, speed);
        DashEnchant.getStringEnchants().put("randomspeed", speed);
        if(debug()){ getLogger().info("RandomSpeed enchant registred"); }

Not that presentable.. uh? How i would literally shorten it? I don't want 350 lines like that.

Answer 1

Assuming you already have a class called Enchant (should be a superclass of your enchantments), you could use an enum to organize all of your enchantments:

public enum Enchantments {
    MULTI_FIRE(1, "multifirearrow") {
        Enchant getEnchant() { return new MultiFireArrow(); }
    }, BOLT(2, "bolt") {
        Enchant getEnchant() { return new Bolt(); }
    }, RANDOM_SPEED(3, "randomspeed") {
        Enchant getEnchant() { return new SpeedRandom(); }
    };
    //Add more enchantments here!
    abstract Enchant getEnchant();
    public void enchant() {
        Enchant enchant = getEnchant();
        DashEnchant.getEnchants().put(this.index, enchant);
        DashEnchant.getStringEnchants().put(this.name, enchant);
        if(debug()) getLogger().info(this.toString() + " enchant registered");
    }
    public boolean isEnabled() {
        return getDashConfig().getBoolean("enchantments." + this.name + ".enabled");        
    }
    private String name;  
    private int index;    
    private Enchantment(int index, String name) {
        this.index = index;
        this.name = name;
    }
}

and then do something like:

for(Enchantment e : Enchantment.values())
    if(e.isEnabled()) e.enchant();

You could make the code even shorter using reflection and enum ordinals but there is a point where shortening the code doesn't really help anymore (plus the added speed penalty.) Nonetheless: (replace MY_PACKAGE_NAME with your package)

public enum Enchantment {
    MULTI_FIRE, BOLT, SPEED_RANDOM;
    //Add more enchantments here!
    public void enchant() {
        Enchant enchant = Class.forName(MY_PACKAGE_NAME +
                toUpperCamel(this.toString(), "_")).newInstance();
        DashEnchant.getEnchants().put(this.ordinal() + 1, enchant); //ordinal starts at 0
        DashEnchant.getStringEnchants().put(this.replaceAll("_", "").toLowerCase(), enchant);
        if(debug()) getLogger().info(this.toString() + " enchant registered");
    }
    public boolean isEnabled() {
        return getDashConfig().getBoolean("enchantments." +
                this.toString().replaceAll("_", "").toLowerCase() + ".enabled");        
    }
    private static String toUpperCamel(String a, String spl) {
        StringBuilder b = new StringBuilder(a.length());
        for(String s : a.split(spl)) {
            b.append(Character.toUpperCase(s.charAt(0)));
            b.append(s.substring(1).toLowerCase());
        } return b.toString();
    }
}

Answer 2

You could put similar code in a method:

void addEnchant(int no, String name, Class<?> clazz)
        throws InstantiationException, IllegalAccessException {
    if(getDashConfig().getBoolean("enchantments." + name + ".enabled")){
        Object enchant = clazz.newInstance();
        DashEnchant.getEnchants().put(no, enchant);
        DashEnchant.getStringEnchants().put(name, enchant);
        if(debug()){
            getLogger().info(clazz.getName() + " enchant registred");
        }
    }
}

Then use it like this:

addEnchant(1, "multifirearrow", MultiFireArrow.class);
addEnchant(2, "bolt", Bolt.class);
addEnchant(3, "randomspeed", RandomSpeed.class);

Answer 3

First step : use a private method and exploit much more the dashconfig object to store and retrieve all required information related to the enchantment.

The idea is filling more properties information in dashconfig and creating a method with a single parameter which is the name of enchantement you try to initialize.
From this name, the helper method will retrieve all associated value :

• the boolean enabled
• the full name of the Enchantment class to instantiate (so instantiation by reflection)
• the number associated to the enchantment.

The initAndConfig() method could be like that :

public void initAndConfigureEnabledEnchantments(String enchantmentName)


 String enchantmentProperty = enchantmentName  + ".enchantments";

    // fail fast
    if(!getDashConfig().getBoolean(enchantmentProperty + ".enabled")){
      return;
    }


   String classFullQualified =           
       getDashConfig().getString(enchantmentProperty + ".class");
   int number = getDashConfig().getInteger(enchantmentProperty + 
           ".number");
    Class<? extends Enchantment> newInstanceClass = (Class<? extends Enchantment>) Class.forName(classFullQualified );
    Enchantment newInstance = newInstanceClass.newInstance();   
    DashEnchant.getEnchants().put(number, newInstance );
    DashEnchant.getStringEnchants().put(enchantmentName, newInstance);

    if(debug()){
      getLogger().info("enchantment " + enchantmentName + " registred");
    }
 }

And you could call it like that :

initAndConfigureEnabledEnchantments("multifirearrow"); initAndConfigureEnabledEnchantments("bolt"); initAndConfigureEnabledEnchantments("randomspeed");

Answer 4

I'd do something like this:

If MultiFireArrow , Bolt and SpeedRandom has a superclass or a common interface you can use it otherwise add an interface or superclass (based on your needs)

In my example I used InterfaceObj an interface implemented in all three classes.

then

setParam(getDashConfig().getBoolean("enchantments.multifirearrow.enabled"), new MultiFireArrow(), 1, "multifirearrow", "MultiFireArrow");
setParam(getDashConfig().getBoolean("enchantments.bolt.enabled"), new Bolt(), 2, "bolt", "Bolt");
setParam(getDashConfig().getBoolean("enchantments.randomspeed.enabled"), new SpeedRandom(), 3, "randomspeed", "RandomSpeed");

public void setParam(boolean checks, InterfaceObj iob, int index, String text, String log)
{
    if(checks)
    {
        DashEnchant.getEnchants().put(index, iob);
        DashEnchant.getStringEnchants().put(text, iob);
        if(debug()){
            getLogger().info(log + " enchant registred");
        }
    }
}

if your getBoolean is always in the format: enchantments.<ACTION>.enabled you can move the getter into the setParam method.

setParam("multifirearrow", new MultiFireArrow(), 1, "multifirearrow", "MultiFireArrow");
setParam("bolt", new Bolt(), 2, "bolt", "Bolt");
setParam("randomspeed", new SpeedRandom(), 3, "randomspeed", "RandomSpeed");

public void setParam(String checks, InterfaceObj iob, int index, String text, String log)
{
    if(getDashConfig().getBoolean("enchantments."+checks+".enabled"))
    {
        DashEnchant.getEnchants().put(index, iob);
        DashEnchant.getStringEnchants().put(text, iob);
        if(debug()){
            getLogger().info(log + " enchant registred");
        }
    }
}