Solved Should I extend Player or use a wrapper?

Big design problem that I cannot figure out the best way to handle. I'm making a Paintball plugin. I have a class called PbPlayer, it is a Player who is in a Paintball Arena. Currently I have it set up by a wrapper, meaning I use the Player instance in the PbPlayer's paramters in order to access the Player classes methods. Although this is already setup, I have found a cleaner way, but should I change it? Is it even cleaner?

So here is how it is right now... (Without extending Player)

Code:

public class PbPlayer {

    Player player;
    ArenaManager.Team team;
    Arena arena;

    /**
     * This will get called everytime a player is added into arena, we create a PbPlayer and
     * add checks on their join based on config settings and values.
     * @param p Player to make a PbPlayer
     * @param t Team of the Player
     * @param a Arena they are joining
     */
    public PbPlayer(Player p, ArenaManager.Team t, Arena a) {
        this.player = p;
        this.team = t;
        this.arena = a;

        initPlayer();
    }

    public String getName() {
        return player.getName();
    }

    public void addHelmet() {
        if (!Settings.WOOL_HELMET) {
            return;
        }

        DyeColor color = team == ArenaManager.Team.BLUE ? DyeColor.BLUE : DyeColor.RED;
        String name = color == DyeColor.BLUE ? ChatColor.BLUE + "Blue Helmet": ChatColor.RED + "Red Helmet";

        // Set the name of the helmet
        ItemStack wool = new Wool(color).toItemStack();
        ItemMeta woolMeta = wool.getItemMeta();
        woolMeta.setDisplayName(name);
        wool.setItemMeta(woolMeta);

        player.getInventory().setHelmet(wool);
    }

    public void giveItems() {
        removeItemsToCache();

        // give them snowballs/rifle that is properly named from config
    }

    // todo: make a list of projectiles to pick from, rile, snowball etc and put paramters here
    public void launchProjectile() {
        // todo:make spawn faster
        player.launchProjectile(Snowball.class, player.getVelocity());
    }

    private void initPlayer() {
        addHelmet();
        giveItems();
    }

    private void removeItemsToCache() {

    }

    private void addItemsToPlayerFromCache() {

    }
}

I think the next way I just discovered is a better design (More Object Oriented)? I am going to make a PbPlayer class abstract then create 2 classes that extend PbPlayer, RedPbPlayer and BluePbPlayer. (for everyone who does not know, that means if a class extends PbPlayer they have to use all the methods in the PbPlayer class)

Code:

public abstract PbPlayer extends Player {

    public abstract giveWoolHelmet();

    public abstract void initPlayer();

    public abstract String getName(); // super.getName()
    public abstract void giveItems();

   public abstract void launchProjectile();

   // etc... I just made this really fast for the thread purpose
}

Then I could have 2 classes, PbRedPlayer and PbBluePlayer using these same methods, both extending PbPlayer, which then extends Player.

I don't know which design choice is better (Extending Player, or using a wrapper)?
I already did it with a wrapper, but I think the second way would be cleaner. I'm not sure, any opinions?

P.S - please ignore the formatting in some of the code and errors, I just made it really quick as an example for this post so you guys got the idea of what I meant...

 

@Synapz I LOVE this question. We need more of these.

I think that I you want easy access to the player's methods, a wrapper class is a much better alternative. Why? You can't simply cast a superclass to a subclass. You would be creating a wrapper anyway. As far as the question of each team, 2 classes for the sane thing is unneccesary and confusing. Adding an enum to the constructor, however, allows you to even add more teams with nothing but a single line in the enum. You could have team data singletons to manage data instead which would make your code far more readable (assuming that isn't already in the arena data)

I'm open to criticism, just no hating please.

 

Thanks!

I don't think I explained about why this maybe better other than the more OOP design? It is for the same thing, but each method would be different... For example the addHelmet method in BluePbPlayer would give a blue helmet, the RedPbPlayer would give a red helmet. Same for items, the RedPbPlayer gets different named items (Red Paintball Gun) opposed to the BluePbPlayer (Blue Paintball Gun), so the classes aren't TOTALLY the same, but I can just add checks in those methods.

But still, you are right! One class is better than three classes. In order to add a Green player I would have to make a WHOLE new class and readd everything to the green helmets, green names blah blah blah fun.... not.

Otherwise with the current method, I am having to create bigger methods with checks in them. Checking each Team and setting the values... For exmaple here:

Code:

 String name = color == DyeColor.BLUE ? ChatColor.BLUE + "Blue Helmet": ChatColor.RED + "Red Helmet";

I guess I will have to do this on each method. But then here's the other thing. If I do add a Green PbPlayer, then I would have to add more checks... I am starting to lead back towards the current option I have.

 

@Synapz, switch statements would be a very efficient way of doing the checks without worrying about performance.

And while the classes are not exactly the same, they are far too similar to do what you are suggesting. It just makes things far more complicated. There are a lot of times when you will learn that somewhat less efficient methods are better than spaghetti code.

 

@Tecno_Wizard Alright, thanks you so much for the help!

 

@Synapz
Please mark thread as solved.
Just a side-note: Player is an interface where you can't just call super on. If you were to extend Player it wouldn't have any effect because the CraftPlayer class handles all the stuff, and you can't simply replace it.

 

@megamichiel, exactly. Your only option is a wrapper.