Am I abusing statics?

Question

To disguise players as another Entity, I made a disguise class, as you can see here:

public class Disguise { private static HashSet<Disguise> disguises = new HashSet<>(); private net.minecraft.server.v1_8_R2.EntityLiving nmsEntity; private Player disguise; public Disguise(Player disguise, EntityLiving entity, boolean affectLogin) { if(affectLogin) disguises.add(this); this.disguise = disguise; this.nmsEntity = entity; } public Disguise(Player disguise, EntityLiving entity) { this(disguise, entity, true); } public void send(Player visible) { if(visible == disguise) return; EntityPlayer player = NMSUtils.getNMSPlayer(visible); nmsEntity.setPosition(player.locX, player.locY, player.locZ); nmsEntity.d(disguise.getEntityId()); nmsEntity.setCustomName(disguise.getDisplayName()); nmsEntity.setCustomNameVisible(true); PacketPlayOutSpawnEntityLiving spawn = new PacketPlayOutSpawnEntityLiving(nmsEntity); PacketPlayOutEntityDestroy destroy = new PacketPlayOutEntityDestroy(disguise.getEntityId()); player.playerConnection.sendPacket(destroy); player.playerConnection.sendPacket(spawn); } public void send(List<Player> visible) { for(Player player : visible) send(player); } public void send(Player... visible) { send(Arrays.asList(visible)); } public void send() { send(new ArrayList<>(Bukkit.getOnlinePlayers())); } public Player getDisguised() { return disguise; } public static HashSet<Disguise> getDisguises() { return disguises; } } 

I also have a static HashSet that stores all instances made. I do this because I want the players who are logged in to also see the disguise, and I want to remove the disguise from the player when the player leaves the game. Is a static HashSet way to do this (how do I do it)? And if not, how to do it?

Answer 1

static requested this. By nature, he is subject to “ill-treatment,” but this is only part of the problem.

When everything is said and done, if your mod does what you need to do without errors, do not emphasize the best practices at this level of detail too much (specific variable). It is unlikely to ever scale to such an extent that poor design will cause problems for you. After all, it is not a life support system.

If you want to practice in good shape for fun, my first instinct would be to move your control logic from the Disguise class to a class (e.g. DisguiseManager) and handle all masking creation / destruction through the manager class. Private constructor and static methods of creation / destruction on camouflage would be less complicated. Global side effects in constructors like you posted are usually bad.

Answer 2

Basically, every time a constructor is called, you want to add this to a global location.

This is good, but there are two problems:

• exposing this in the constructor is dangerous and requires careful analysis. (your code does not work in this aspect)
• concurrency - if the application is multithreaded, it should be thread safe. (exposing this in the constructor is more problematic in a parallel environment)
• garbage collection - when an object becomes “garbage”, how to remove it from a global location.

Answer 3

Using static objects can become really frustrating when your code grows in size and there are many accessories of the specified object. If you want to debug code, how do you catch the exact code for managing a HashSet?

Why not reorganize your clients using a HashSet to get it through a getter? How about encapsulating a HashSet instance like Singleton ? It looks like only one HashSet is created to hold players / Disguies.

Having any getter method, say, through Singleton, you can easily add additional code before or after accessing the HashSet. For example, after each use of the method returning a HashSet, you can print the contents of the HashSet. You could do this with a static object as well, but a nightmare to find all uses of a static object ...