[Deal™]

This is all Bukkit but that's kind of irrelevant to the topic. I have three classes who purpose is to certain objects (inventories) depending on which group a player is in.

Code

    public void loadAbilityBar(Player p, boolean forceReload){
        //Space Marine
        if(ThunderSuite.perms.getPrimaryGroup(p).equals("Scout")){
            loadArray(p, aa.scoutAbilities, forceReload);
        }
        else if(ThunderSuite.perms.getPrimaryGroup(p).equals("SpaceMarine")){
            loadArray(p, aa.spaceMarineAbilities, forceReload);
        }
        else if(ThunderSuite.perms.getPrimaryGroup(p).equals("AssaultMarine")){
            loadArray(p, aa.assaultMarineAbilities, forceReload);
        }
        else if(ThunderSuite.perms.getPrimaryGroup(p).equals("GreyKnight")){
            loadArray(p, aa.greyKnightAbilities, forceReload);
        }
        else if(ThunderSuite.perms.getPrimaryGroup(p).equals("Terminator")){
            loadArray(p, aa.terminatorAbilities, forceReload);
        }
        else if(ThunderSuite.perms.getPrimaryGroup(p).equals("AssaultTerminator")){
            loadArray(p, aa.aTerminatorAbilities, forceReload);
        }
        else if(ThunderSuite.perms.getPrimaryGroup(p).equals("ForceCommander")){
            loadArray(p, aa.forceCommanderAbilities, forceReload);
        }
        
        //Chaos
        else if(ThunderSuite.perms.getPrimaryGroup(p).equals("Cultist")){
            loadArray(p, aa.cultistAbilities, forceReload);
        }
        else if(ThunderSuite.perms.getPrimaryGroup(p).equals("ChaosMarine")){
            loadArray(p, aa.chaosMarineAbilities, forceReload);
        }
        else if(ThunderSuite.perms.getPrimaryGroup(p).equals("ChaosRaptor")){
            loadArray(p, aa.chaosRaptorAbilities, forceReload);
        }
        else if(ThunderSuite.perms.getPrimaryGroup(p).equals("Horror")){
            loadArray(p, aa.horrorAbilities, forceReload);
        }
        else if(ThunderSuite.perms.getPrimaryGroup(p).equals("KhorneBerserker")){
            loadArray(p, aa.khorneBerserkerAbilities, forceReload);
        }
        else if(ThunderSuite.perms.getPrimaryGroup(p).equals("PossessedMarine")){
            loadArray(p, aa.possessedMarineAbilities, forceReload);
        }
        else if(ThunderSuite.perms.getPrimaryGroup(p).equals("ChaosLord")){
            loadArray(p, aa.chaosLordAbilities, forceReload);
        }
        
        //Ork
        else if(ThunderSuite.perms.getPrimaryGroup(p).equals("Gretchin")){
            loadArray(p, aa.gretchinAbilities, forceReload);
        }
        else if(ThunderSuite.perms.getPrimaryGroup(p).equals("SluggaBoy")){
            loadArray(p, aa.sluggaAbilities, forceReload);
        }
        else if(ThunderSuite.perms.getPrimaryGroup(p).equals("ShootaBoy")){
            loadArray(p, aa.shootaAbilities, forceReload);
        }
        else if(ThunderSuite.perms.getPrimaryGroup(p).equals("StormBoy")){
            loadArray(p, aa.stormBoyAbility, forceReload);
        }
        else if(ThunderSuite.perms.getPrimaryGroup(p).equals("Nob")){
            loadArray(p, aa.nobAbilities, forceReload);
        }
        else if(ThunderSuite.perms.getPrimaryGroup(p).equals("FlashGit")){
            loadArray(p, aa.flashGitAbilities, forceReload);
        }
        else if(ThunderSuite.perms.getPrimaryGroup(p).equals("WarBoss")){
            loadArray(p, aa.warBossAbilities, forceReload);
        }
    }

That method has even more else-ifs but I cut it off for readability. Obviously I could also use a switch statement but that still seems like a ridiculous amount of code to achieve what I need.

Code

    public void loadArray(Player p, Ability[] array, boolean forceReload){
        Inventory inv = toolbars.get(p.getName());
        if((inv == null) || (ThunderSuite.perms.getPrimaryGroup(p) != JoinListener.groups.get(p.getName())) || forceReload){
            inv = Bukkit.createInventory(p, 9, ChatColor.DARK_BLUE + "Abilities");
            for(int i = 0; i < array.length; i++){
                inv.setItem(i, array[i].getItem());
            }
            JoinListener.groups.put(p.getName(), ThunderSuite.perms.getPrimaryGroup(p));
            toolbars.put(p.getName(), inv);
        }
        p.openInventory(inv);
    }

That method loads an inventory with the necessary items as defined by the array parameter and presents it to the player.

Code

public class AbilityArrays {
    
    StrafeRun sr = new StrafeRun();
    JetPack jp = new JetPack();
    Infiltration inf = new Infiltration();

    Zeal zeal = new Zeal();
    LiberDaemonicus lb = new LiberDaemonicus();
    TacticalDreadnoughtArmour tda = new TacticalDreadnoughtArmour();
    CruxTerminatus ct = new CruxTerminatus();
    ThunderHammer th = new ThunderHammer();
    
    MarkOfChaos moc = new MarkOfChaos();
    WarpFlames wf = new WarpFlames();
    BloodRage br = new BloodRage();
    RuinousPowers rp = new RuinousPowers();
    Tendrils t = new Tendrils();
    
    ToughSkin ts = new ToughSkin();
    Slugga sl = new Slugga();
    Shoota sh = new Shoota();
    PowerClaw pc = new PowerClaw();
    KustomShoota ks = new KustomShoota();
    Roks r = new Roks();
    
    //Space Marine
    public Ability[] scoutAbilities = new Ability[]{
        sr,
        inf
    };
    
    public Ability[] spaceMarineAbilities = new Ability[]{
        zeal
    };
    
    public Ability[] assaultMarineAbilities = new Ability[]{
        jp
    };
    
    public Ability[] greyKnightAbilities = new Ability[]{
        lb    
    };
    
    public Ability[] terminatorAbilities = new Ability[]{
        tda,
        ct
    };
    
    public Ability[] aTerminatorAbilities = new Ability[]{
        tda,
        ct
    };
    
    public Ability[] forceCommanderAbilities = new Ability[]{
        th
    };
    
    //Chaos
    public Ability[] cultistAbilities = new Ability[]{
        inf
    };
    
    public Ability[] chaosMarineAbilities = new Ability[]{
        moc
    };
    
    public Ability[] chaosRaptorAbilities = new Ability[]{
        jp
    };
    
    public Ability[] horrorAbilities = new Ability[]{
        wf
    };
    
    public Ability[] khorneBerserkerAbilities = new Ability[]{
        br
    };
    
    public Ability[] chaosLordAbilities = new Ability[]{
        rp
    };

    public Ability[] possessedMarineAbilities = new Ability[]{
        t
    };
    
    //Orkz
    public Ability[] gretchinAbilities = new Ability[]{
        inf
    };
    
    public Ability[] sluggaAbilities = new Ability[]{
        ts,
        sl
    };
    
    public Ability[] shootaAbilities = new Ability[]{
        ts,
        sh
    };
    
    public Ability[] stormBoyAbility = new Ability[]{
        jp
    };
    
    public Ability[] nobAbilities = new Ability[]{
        pc
    };
    
    public Ability[] flashGitAbilities = new Ability[]{
        ks
    };
    
    public Ability[] warBossAbilities = new Ability[]{
        r
    };

}

Finally that class contains all of the arrays for each group, and yes I'm well aware that many of my Abilities need renamed since some of them are literally just one character. 😝 Surely there is a way to size this shit down.

[rickhunter]

For your inventory arrays you can store them in a tree map and use the string returned from

PHP Code


<?php 
ThunderSuite.perms.getPrimaryGroup(p); 


to find the correct array all in one line.

PHP Code


<?php 
String s = ThunderSuite.perms.getPrimaryGroup(p);
TreeMap tm = new TreeMap(String,Array); // psuedo code this would be constructed before hand and filled with every value

Array[] a = tm.get(s); // finds the value mapped to the string and returns it

loadArray(a); 


This is assuming of course you know what a treemap is and how to use it. If not just let me know and I can explain in more detail.

With this you can kill all your else if and condense it into one line.

[Deal™]

I haven't had to use a treemap before but it looks like it behaves pretty similarly to a hashmap so I bet I can figure it out.

[rickhunter]

I haven't had to use a treemap before but it looks like it behaves pretty similarly to a hashmap so I bet I can figure it out.

Yepyep they are both maps (obviously 😝).
Let me know if you need some help.

Edit: I can probably come up with some more stuff but I'm about to pass out. If no one else has answered tomorrow I'll post some more.

[Psifour]

Umm, at a glance your code needs some serious life-support. If you ever have more than three or four cascading if/else statements you need to rethink your design/implementation. Second issue I see is that you could store all this information (and access it easily and efficiently) with two enums and a HashMap, but instead you wrote it all long-hand with no care for the amazing tools that Java provides for you.

Now, I know I am being needlessly harsh so I will provide you an option I usually wouldn't for plugin related code. Push your code to github and PM me a link. I will restructure/refactor your code for efficiency and readability (with the focus on the latter).

[Deal™]

Yepyep they are both maps (obviously 😝).
Let me know if you need some help.

Edit: I can probably come up with some more stuff but I'm about to pass out. If no one else has answered tomorrow I'll post some more.

I changed it to use a treemap and it certainly looks much cleaner. I assume that a treemap seaches through larger map quicker than a hashmap would. If not then I have no idea and guess I'll have to look into it more. I'm going to bed too but I'm still open to suggestions and I'll be sure to check back in the morning. Thanks for your insight.

Umm, at a glance your code needs some serious life-support. If you ever have more than three or four cascading if/else statements you need to rethink your design/implementation. Second issue I see is that you could store all this information (and access it easily and efficiently) with two enums and a HashMap, but instead you wrote it all long-hand with no care for the amazing tools that Java provides for you.

Now, I know I am being needlessly harsh so I will provide you an option I usually wouldn't for plugin related code. Push your code to github and PM me a link. I will restructure/refactor your code for efficiency and readability (with the focus on the latter).

You have no idea how much I would value insight like that from someone like you. I will be sure to take you up on that in the mourning!

[Sparta.]

I'm sorry, but I got cancer from reading that code. Please research OOP and practice it.

[-Base-]

I agree, the statements are kind of all over the place, specifically the first and last snippet of code you posted. You can take some of what you have and make it more object oriented so that you don't need as much code. If you send me a PM I'd be more than happy to help you clean up this code.

[Psifour]

I'm sorry, but I got cancer from reading that code. Please research OOP and practice it.

Sparta, you are always so blunt and straight-forward. It is both entertaining and refreshing.