Static plugin instance

Hey guys, I've wondered if it is more recommendable to use a static plugin instance instead of handing over the instance of it in classes which need to access it. I'm currently not using a static plugin instance, but I tend to use one so please tell me if there are any disadvantages or notable things about that.

 

No, i wouldn't recommend it at all.

- http://stackoverflow.com/a/138012/1786065

 

1Rogue Hmm that seems logic, but why do big plugins like WorldGuard use it then?

 

They are most likely used as accessors rather than direct dependencies for code. Although, if they are using them for dependencies in code then I would say that they are still doing a bad practice. Regardless of how big a plugin is, there is always something that can be improved.

 

1Rogue Okay, thanks for telling me. I'm going to continue handing over the instance of my plugin!

 

It's arguable. One of those things where each developer has their own opinion. I disagree with 1Rogue in this case, with the exception that if you don't know when to properly execute a static statement it can lead to trouble.

Younger developers have difficulty determining what should be static and what should be instanced so they do one or the other. While having everything static typically produces compiler warnings and errors, having everything instanced, even when it's the same code regardless of instance, is a complete waste of memory and creating the objects to execute a "should-be-static" statement takes a lot of (relative) time. It's important to learn when to recognize methods that do not access anything "unique" to any specific object in order to maintain efficiency.

When I execute a static instance of the plugin, I generally do it as a return to resolve a lot of the issues associated with storing a plugin in a variable. This leaves it "static but not persistent".

Code:java

public static MyClass getInstance() {
return (MyClass)Bukkit.getPluginManager().getPlugin("MyPlugin");
}

 

Which is what a lot of plugins do (I had this in a few for a while as well). Overall, static (variables) create persistent objects throughout the runtime of your code (which is bad if it is unnecessary, re: waste of memory). As far as static methods go there is a place for them, but bukkit plugins are typically not one of them. Really the only things that should be static singletons are things that either need to have only a single instance (rare), or something that would require essentially one-way traffic.

A good example of singleton usage is Loggers. You don't want to log to multiple things, and all things feed into this single logger but the code itself isn't dependent on the functionality of this logger.

 

And why would you want to have multiple instances if your main plugin class? Bukkit plugins are essentially Singletons, there is not really a point in avoiding singleton-specific methods, as long as you get the right.

 

Garbage collection of singleton instances on server reloads comes to mind (although the reload method is already pretty iffy).

 

The main class instance in Bukkit plugins would seem to be a great example of using static. It's an instance widely used throughout the classes in a plugin and making a ton of constructors is a pain.

 

The_Doctor_123 So does the plugin unload properly with a static instance of itself or does it stay in memory then?

 

DarkBladee12
Good question.. I'm not exactly sure how the unloading works. Possibly the GC might pick it up, I'm not sure. You could be safe and nullify the reference in the onDisable() method.

 

Yes, you should null all static variables on onDisable() to speed up reloads as well as lower the amount of memory taken up. That said, unless you have a near or completely custom plugin setup, you really shouldn't reload as a lot of plugins don't handle it correctly and it's very slow and eats up memory.

 

Garris0n Okay, so all in all using a static plugin instance is totally safe and not bad programming?

 

Nothing is totally safe, but there's a reason that all java programs start with static void main

 

Garris0n
I didn't know that !, well atleast ive learned something

 

If you like memory leaks through being unable to GC a static class instance, perfectly safe.

Static itself isn't bad practice. Misuse of it is.

True enough, but there's a difference between a static method and variable.

Another thing (unrelated to the post above) to note is that you can set a static variable to null but there's still no guaruntee that the garbage collector will actually collect it.

As far as storing, the implementation of the static plugin var would be whether it serves as a reference to the plugin object, or whether the static variable is actually the value of the class. (difference between young/new gen and being stored in the perm gen)

EDIT by Moderator: merged posts, please use the edit button instead of double posting.

 

My unrequested opinions...

Static classes can be good for wrapping up a set of utility functions that don't require persistent state data. If you make such a static class, and suddenly find yourself adding static properties (not just methods), ask yourself if it should be an object now.

Static classes are also a way to represent the Singleton pattern, though just making a static class with static properties and methods is not the best way to do that. The singleton pattern is usually best represented as a normal class with a private constructor and a single static instance to itself, accessed via a static getInstance() method that creates the instance on demand. This helps solve 1/2 of the lifecycle problem of statics- you at least now control creation (though it will still live forever...).

Example of how you'd use such a thing if this is not familiar:

Code:java

// MySingleton is a singleton class
// It has only one static method, getInstance
// This method always returns the same instance of the class
// no matter how many times it is called.
// This instance is created on demand.
 
MySingleton instance = MySingleton.getInstance();
instance.foo();
 
// MySingleton would look something like
public class MySingleton {
  private static MySingleton instance;
  private MySingleton() {
    // Construct is private, you can't create one
  }
 
  public static MySingleton getInstance() {
    if (instance == null) instance = new MySingleton();
    return instance;
  }
 
  public void foo() {
    // something you want your singleton to do
  }
}

When to use a singleton is a topic in itself, but in my experience it's best for "systems" that you want to treat like a single instance (e.g. a database management system or something). And most often this is a system that, once started, does not terminate until the main process is done.

So, tying this all together in a Bukkit frame of mind- a plugin is kind of a singleton already (just.. due to the nature of how Bukkit uses it), but keeping your own static references to it is a bad idea unless you're sure you're going to be sure you can clean them all up on disable. This relates to the "does not terminate until the main process is done" bit above.

Basically the lifecycle of your plugin object is not under your control. Do not assume that, once started, your plugin is meant to stay in memory forever. An admin may decide to disable it, at which point it had better go away- and it probably won't if you've got static instances to it somewhere.

The solution I normally use to this is to pass the plugin instance along to constructors when needed (not ideal, I know). My code is structured in a parent/child sort of way, each class has a reference to its parent - so no matter how "deep" I am I can usually get back to the plugin if needed (and it's not really needed that often, but dunno what you're doing). This means I don't really have to pass the plugin to everything, just the first level of classes in my hierarchy, the things directly created by the plugin itself.

Another option is making a static method on your plugin that calls PluginManager.getPlugin each time, rather than caching an instance. That ought to just be a map lookup, so relatively efficient as long as you're not calling it constantly. At that point it's really just a utility wrapper for getting the PM from Bukkit and calling getPlugin.

Ok, long-winded...
Hope some of that helps!

 

NathanWolf

I would say when you're optimizing, passing a reference is usually cleaner. I personally pass the top of my hierarchy down instead of chaining. This could be bad in a sense of the main class becoming a "God class", but overall seems the cleanest IMO.

 

NathanWolf sadly very few plugins on bukkit are properly designed to be disabled without reloading. Static variables asside, I doubt many people properly unregister their events.

 

I've noticed that even if you don't unregister they don't double, so I assume that the HandlerList is cleared on a /reload.

https://github.com/1Rogue/PlayTime/blob/master/src/main/java/com/rogue/playtime/Playtime.java#L183

This code is only necessary if you are reloading within yourself, really.

 

1Rogue Yes, but he was talking about disabling a plugin without reloading. unregisterAll() is a good way to do that, but many don't.

 

Er... yeah, I don't! I thought that happened automatically on disable... isn't that why you pass the plugin instance when registering?

But yeah in general I think you're probably right.. I try to handle enable/disable/reload, but it gets tricky sometimes.

I was hesitant to even mention how I was doing it since in general you're probably right. If you have some object that you need to access "everywhere", then just pass the instance around.

But in my specific case each part of the hierarchy needs a reference to its parent anyway (which is also not the best design pattern, but.. eh) so in the few, rare cases where I need to get the plugin instance from deep in my code, walking back up the hierarchy makes sense.

In general though I try to keep code that needs to do plugin stuff in my plugin's main class- and that's all I have in there. It's pretty rare that some class I have will try to add/remove an event handler, or do some other thing that requires a plugin reference... but maybe I'm the exception there.

 

It might, but it isn't guaranteed. At that level we are talking outside the Bukkit API, so CraftBukkit might do it, but does Spigot or MCPC+? You just really don't know, it's best to handle it yourself and cover your bases.

EDIT: actually I may be mistaken. That is in the API but I don't see where it unregisters events. Still looking.

EDIT2: Oh! it does. Look at that, learn something new every day.

 

So from all what I've read I think it's better to pass the instance of the plugin only to the classes which need to access it than making a static instance. Thanks for all answers and explanations NathanWolf 1Rogue Garris0n The_Doctor_123

 

DarkBladee12
It's not bad unless you don't know what you're doing.