[New Devs] How to make your plugin right/Make it better

Discussion in 'Resources' started by teej107, Oct 16, 2014.

Thread Status:
Not open for further replies.
  1. Offline

    teej107

    Maybe I lost patience, maybe this is for the better, but I see myself saying the same thing over and over to new developers when I could just point them to some thread that explains everything rather than having to repeat myself over and over again.

    ___________________________________________________________________
    Learn Java First!

    I say this in almost every new developer's thread that they post. Especially if this is their first programming language.

    Becoming sufficient in Java doesn't happen over night. It'll take months to get a grasp of it. This includes the syntax, the libraries, etc. I not saying to not learn Java with Bukkit, but keep in mind that Bukkit is just an API. When I started learning the Bukkit API, I noticed it had some ways of doing things that you wouldn't see in the Java API. The most important thing I can say about this is to be patient. You'll get better over time with the more knowledge of the libraries and techniques to achieve certain outputs.

    ___________________________________________________________________
    Bad Code

    There are many ways to learn Java through different forms of media. Each person has their own preferred way of learning (spoonfeeding isn't learning, you'll get a better knowledge if you try things out yourself. eg: trial and error). Be aware though that some of these methods of learning aren't great. Some teach bad coding practices and/or is outdated.

    Official tutorials are pretty good: http://docs.oracle.com/javase/tutorial/index.html

    For new developers, how can you tell if the code is bad? Well one way is getting proficient at the language. However if you did that, I wouldn't consider you to be a new developer. I'll show some tips to make your code readable and perform well.

    First off, if your plugin looks anything like this, delete it, throw it away, ignore it, I don't care what you do with it. It's code not worth looking at.
    Code:
    package me.Teej107.MyPlugin;
    
    import java.util.logging.Logger;
    
    import org.bukkit.command.Command;
    import org.bukkit.command.CommandSender;
    import org.bukkit.entity.Player;
    import org.bukkit.plugin.java.JavaPlugin;
    
    public class Main extends JavaPlugin
    {
        public final Logger logger = Logger.getLogger("Minecraft");
        public static Main plugin;
    
        public void onEnable()
        {
            logger.info(this.getName() + " has been enabled!");
            plugin = this;
        }
    
        public void onDisable()
        {
            logger.info(this.getName() + " has been disabled!");
        }
    
        @Override
        public boolean onCommand(CommandSender sender, Command command,
        String label, String[] args)
        {
            Player player = (Player) sender;
            if(label.equalsIgnoreCase("test"))
            {
                player.sendMessage("This is a test!");
            }
            return true;
        }
    }
    
    From top to bottom, I'll show you the mistakes made in a plugin like this one.

    ___________________________________________________________________
    Naming Conventions
    The package name:
    Code:
    package me.Teej107.MyPlugin;
    This is a useful thread to read about package names: http://forums.bukkit.org/threads/on-namespaces-please-do-not-use-bukkit-in-your-plugins.3732/.
    It is extremely important to read about the Java Naming Conventions. Java Naming Conventions doesn't affect the code at runtime, but think of it as grammar for Java. Good grammar makes it a lot easier to understand and read. BAd Gramm3r is rly hard, to. understand.
    This is the correct way to write the package name: me.teej107.myplugin.

    The class name:
    Main is a bad generic name for a class name. Every plugin could use Main as their "Main" class and it would get confusing. The Java Naming Conventions state:
    Instead, have the name of your "Main" class be the title of your plugin / or what the plugin does. If a plugin's purpose allows for customized achievements, the "Main" class could be named CustomAchievements.

    ___________________________________________________________________
    The Logger
    Here is the documentation for the method to obtain the correct logger. JavaPlugin#getLogger()
    To sum this up, stop stealing Minecraft's logger. It is Minecraft's logger and not the plugin's.

    ___________________________________________________________________
    Encapsulation & Static Keyword
    Code:
    public static Main plugin;
    This line is a big no-no. First no-no is the public modifier. Ever heard of encapsulation? Encapsulation not only hides information, it makes it more readable. If you want to access variables from an object, use what is known as getters.

    Secondly, the keyword static isn't a magic solution to access information from another class. I doubt the term singleton comes to mind when you do that. Static shouldn't be handled carelessly.
    It is also wise for static declarations to be made immutable (final). I see so many programming errors (NullPointerExceptions) because people do not assign an actual value to their variables (also because they are using it badly). There are valid ways to use static, but for new developers, I would stay away from static and get used to the concept of Object Oriented Programming.

    ___________________________________________________________________
    Accessing information from other classes

    http://docs.oracle.com/javase/tutorial/java/javaOO/arguments.html.
    ___________________________________________________________________
    "Plugin has been enabled"
    Code:
    logger.info(this.getName() + " has been enabled!");
    Did you know that Bukkit already prints to the console when a plugin is enabled/disabled? No need to do it yourself. It's just extra spam in the console.

    ___________________________________________________________________
    Inheritance!
    http://docs.oracle.com/javase/tutorial/java/concepts/inheritance.html
    Code:
    @Override
    public boolean onCommand(CommandSender sender, Command command,
    String label, String[] args)
    {
        Player player = (Player) sender;
        if(label.equalsIgnoreCase("test"))
        {
            player.sendMessage("This is a test!");
        }
        return true;
    }
    Player is an instance of CommandSender. It's the reason why you can cast a CommandSender to a Player. But CommandSender is not a Player. http://docs.oracle.com/javase/tutorial/java/concepts/inheritance.html. From expanding on the link, there are many types of Bicycles, but a tandem bike is not a mountain bike and a mountain bike is not a tandem bike. To check to see if an object is an instanceof a class, simply do this.
    Code:
    if(object instanceof AnotherObject)
    {
        //Cast here
    }
    It does no good to do a cast before an instanceof check.
    onEnable() and onDisable() are overridden methods. Please put @Override annotation over them to save yourself from a possible headache.
    That is all.

    ___________________________________________________________________
    Command Aliases
    Command.getName() vs commandLabel. Be aware that you lose support for command aliases when you use commandLabel over Command.getName() when you check for commands in the onCommand() method. It is a good habit to just use Command.getName() so just use it.


    Other helpful sources: http://forums.bukkit.org/threads/how-to-make-your-plugin-better.77899/

    I appreciate constructive criticism. If you think I need to add something, speak up.

    EDIT by Moderator: merged posts, please use the edit button instead of double posting.
     
    Last edited by a moderator: Jun 14, 2016
  2. Offline

    Jaaakee224

    1. onDisable do <name> = null;
    2. Having { on the same line is unnecessary., but keeps code clean. Having the curly braces on the same line is good for new developers.
    3. I'm pretty sure that at the end of commands it is return false; Don't quote me on this.
     
  3. Offline

    teej107


    1.
    I still prefer to access information the OOP way.
    2. There are many different indent styles. Unless there is something in the Java Naming Conventions for a preferred indent style, I will continue to use the style that I prefer.
    3. returning true doesn't display the usage to the user. returning false does. I didn't really care about that part of my rant as it is not that important in this thread.
     
  4. Offline

    Jaaakee224

    Remember that new developers may use this. So tell them other options as well, so they don't think they are forced to write the exact same code as here. Provide other tutorials as well.
     
  5. Offline

    teej107

    Jaaakee224 I did mention that there are other valid ways of using static (and I did post a few), but I feel that new devs should know how OOP works since Java is an OOP language. I did advise new programmers to learn Java first, then the Bukkit API. I also gave links to some tutorials. What other tutorials were you thinking of?
     
  6. Offline

    Jaaakee224

    teej107
    Basic things such as Using Multiple Classes, etc. Maybe list some good tutorials on Bukkit on youtube. Also talk about memory leaks if you haven't already. (I read through really fast, may not have seen it if you did say anything about memory leaks.)
     
  7. Offline

    teej107

    Jaaakee224 I did give a quick tutorial (and link to one) on how to pass information through to other classes. When somebody takes my advice, there shouldn't be any memory leaks, since I advised not to make a static instance of a plugin.
     
  8. Offline

    croc122

    These are some nice tips teej107 They helped me a lot!
     
    teej107 likes this.
  9. Offline

    _Filip

  10. Offline

    xTrollxDudex

    Rocoty likes this.
  11. Offline

    RawCode

    there are oracle coding conventions posted almost 12 years ago that cover all possible questions about "how to name methods and stuff".

    you are 12 years late.
     
  12. Offline

    Dudemister1999

    You know what bothers me? When someone's listener registration code looks like this:

    Code:java
    1. @Override
    2. public void onEnable()
    3. {
    4. Bukkit.getPluginManager().registerEvents(this, this);
    5. Bukkit.getPluginManager().registerEvents(new Event(), this);
    6. Bukkit.getPluginManager().registerEvents(new Event2(), this);
    7. }


    Here's mine!

    Code:java
    1. @Override
    2. public void onEnable()
    3. {
    4. registerListeners();
    5. }
    6.  
    7. private void registerListeners()
    8. {
    9. PluginManager pm = Bukkit.getPluginManager();
    10.  
    11. pm.registerEvents(this, this);
    12. pm.registerEvents(new PlayerInteractListener(this), this);
    13. }
     
  13. Offline

    teej107

    Dudemister1999 Yeah, your preferred way is definitely a little more organized.
     
  14. Offline

    RingOfStorms

    Honestly that isn't that big of a deal. Generally speaking that is the only thing that is put in the onEnable so really there is much need to make a separate method for those registers.
     
  15. Offline

    Dudemister1999

    RingOfStorms It isn't even that, my issue is all the Bukkit.getPluginManager() calls.
     
  16. Offline

    teej107

    Dudemister1999 You have to do that to get the PluginManager. But any thing that can help me reduce the amount of typing I have to do while still having good code structure I will do.
     
  17. Offline

    RingOfStorms

    What's so bad about it? You're getting the plugin manager. It's not like calling it several times is a performance hit or anything. In the end you've added a line and still have to type pm.regististerEvents every single time.
     
    1Rogue likes this.
  18. Offline

    RawCode

    Dudemister1999

    your sample is completely invalid, you will understand why if ever read any book about "how to code properly"
     
    _Filip likes this.
  19. Offline

    Monkey_Swag

    Tgis is one great, quality tutorial. Great job teej107 ! I've seen newbies abuse their panacea 'static' word to use methods from their main class :p
     
    teej107 likes this.
  20. Offline

    teej107

    Monkey_Swag static methods are fine as long as they don't access anything from the object.
     
  21. Offline

    Dudemister1999

    RawCode RingOfStorms teej107 Don't know if this is correct or not, but throughout my Java readings I've heard at least twice (Shocker, large number) that several static function calls (especially for larger plugins) can reduce speeds. Like I said, don't know if it's correct. But my code is pretty!
     
  22. Offline

    RingOfStorms

    A method being static means just about nothing on the performance level. If there is a difference it is such an incredibly small difference to where it doesn't matter at all. I think you may be mistaken for functions that cause a performance drop and just happen to be static. I also am not sure what the size of a plugin has to do with the performance of one method call.

    Either way, the getPluginManager call is just returning an instance. It doesn't have to search or do anything special to get that instance, so there really is no benefit for performance on this level.
     
    1Rogue likes this.
  23. Offline

    Dudemister1999

    RingOfStorms I do not mean static methods in general, I'm just horrid at explaining myself. Look at my plugin development post history, you'll see a pattern of bad explanation. I don't know, I suppose I just don't believe in repetition in an unsimplified form. But I did hear somewhere that some static methods (I think ones revolving around Reflection) cause performance decreases. But then again, quite a lot of Reflection is not performance-fancy.
     
  24. Offline

    RingOfStorms

    The fact that a method is static doesn't change anything. A method that is static with reflection will have the same performance issues as a non-static method with the same reflection.
     
    1Rogue likes this.
  25. Offline

    RawCode

    Dudemister1999

    Static methods and fields ARE slower then instance methods and fields, but this related ONLY to one more internal lookup from object to it's klass (NOT A TYPO, this is how internal object is called inside JVM source code).
    Performance penalty is so small, that you will need special build of JVM in order to measure processor ticks or invoke method 100000 or more times.

    Many users posted invalid tutorials and stated *facts* they never actually checked, generic rule is simple - if anyone state that something is better without any proof - he lie.

    If you want more info about static vs instance vs array (both methods and field access are actually methods, arrays have field access only, array access is slowest due to 3 lookups, instance is fastest - single lookup) you should start from simplex noise java original source code from developer.
     
  26. Offline

    Dudemister1999

    RawCode I'll have to look into that, thanks. Only reason I believed it is a few people said it. I'll have to see if I can find where, it's been a while.
     
  27. Offline

    d3v1n302418

    Yeah im going to need to copy this link for when I see new devs. Thanks for this teej107 :)
     
Thread Status:
Not open for further replies.

Share This Page