Is this a good or bad static practice?

Discussion in 'Spigot Plugin Development' started by Pannkaka, Apr 18, 2017.

  1. Hello!

    I'm currently developing a fairly large plugin and want to have a easy lang system. At first i though of using a enum to identity and access each lang in the plugin but i didn't like that when i got enum names like "COMMAND_SET_SPAWN_SUCCESS", they got a little bit long and clunky so instead i made this system using final static variables inside of their own static subclass from the parent Lang class.

    Code (Text):
    public class Lang {

        private static List<Lang> langList = new ArrayList<>();

        public static class GENERAL {
            private static final String IDENTIFIER = "general.";

            public static final Lang PREFIX = new Lang(IDENTIFIER, "prefix", "&bHub&3Control &7>>");
            public static final Lang NO_PERMISSION = new Lang(IDENTIFIER, "no-permission", "&cYou do not have the right permission to do this.");
        }

        public static class COMMAND {
            private static final String IDENTIFIER = "command.";

            public static final Lang ONLY_PLAYER = new Lang(IDENTIFIER, "only-player", "&cYou can only run this command as a player.");
            public static final Lang CANNOT_RUN = new Lang(IDENTIFIER, "cannot-run", "&cCannot run this command. Contact a server administrator.");
        }

        private String path;
        private String def;

        public Lang(String identifier, String path, String def) {
            this.path = identifier + path;
            this.def = def;
            langList.add(this);
        }

        public TempLang getLang(CommandSender sender) {
            return new TempLang(sender, null);
        }

        public void sendLang(CommandSender sender) {
            new TempLang(sender, null).sendLang();
        }

        public String getPath() {
            return path;
        }

        public String getDefault() {
            return def;
        }

        public static Lang[] values() {
            return langList.toArray(new Lang[langList.size()]);
        }
    With this way can i access the lang with "Lang.COMMAND.ONLY_PLAYER for example witch i think is a little more cleaner especially with the ability to organize the lang in sub classes.

    This way is clearly working but is this a good thing to use static for?
     
    #1 Pannkaka, Apr 18, 2017
    Last edited: Apr 18, 2017
  2. Static is generally only acceptable for ease of access to values which do not change such as the value of Pi in the math class. eg: Math.Pi

    All other uses are strictly up to the individual programmer and a good rule of thumb is to consider if anyone else will ever be using your code, if so, its good practice to avoid static wherever possible.

    If you cannot think of a clean way to avoid using static, feel free to ask for help as there are a lot of highly skilled programmers on here who specialize in that area.
     
  3. For this case I would use an Enum which automatically uses .toString to calculate the path for the Lang object.

    Each enum would have a constructor to set the default value property. When the enum inits it will also set the property using
    Code (Text):
    public enum CommandValue {

        ONLY_PLAYER("You can only run this command as a player"),
        CANNOT_RUN("Canbot run thiscommand . Contact admin");
     
        private final String actualValue;
        private final Strint defaultValue;
        private static final IDENTIFIER = "command";

        public CommandValue(String defaultValue) {
        String path = super.toString().Replace("_", "-").toLowerCase();

        if isSetInConfig {
            // Create new file and set path to default value and set actual value to default
        } Else {
            // Pull from file with path and then set actualValue to what was pulled from file
        }

        }

        @Override
        public String toString() {
            return this.actualValue;
        }
    }
    Then you just do
    Code (Text):
    print(CommandValue.ONLY_PLAYER);
     
    #3 Synapz, Apr 18, 2017
    Last edited: Apr 18, 2017
    • Agree Agree x 1
  4. Most people will most likely tell you this is disastrous, a terrible practice that should be killed with fire and that shames the keyword 'static'.

    In my opinion it's not that bad, though I have three suggestions:

    1) Use Enums as subclasses;
    2) Make GENERAL and COMMAND interfaces (this is actually a common practice). Of course the purpose is not to implement the interfaces, but it allows you to get rid of the "public static final" keywords (since they are implied when specified in an interface) and cleans up the code a little.
     
    • Agree Agree x 1
  5. This is similar to a lang setup i used in another plugin, but with this way would i need to make a new enum for every lang "type" and copy/paste the core code and i don't really want that.
     
  6. I have seen code where interfaces have been used like that, but is there actually any official code where that method has been used?
     
  7. Option 1:

    Add a Lang variable to store the Lang object for a type... You do not have to make a new enum for every lang type...
    Inside constructor:
    Code (Text):
    this.lang = new Lang(CommandValue.IDENTIFIER, path, defaultValue);
    Then provide an accessor method. Then just use
    Code (Text):
    CommandValue.NOT_PLAYER.getLang()
    And you would not have to manually do anything

    Option 2

    I would just use this enum and merge Lang into the enum and make the enum a little more abstract to cover all the different methods (why separate all the different types?). So the enum class would do everything Lang does.

    For example this is my whole messages.yml class for my Paintball plugin:
    https://github.com/Synapz1/Paintball/blob/master/src/me/synapz/paintball/enums/Messages.java

    Don't really need the Lang object? Otherwise you can keep it as I set in first option
     
  8. Sorry, i misunderstood but yes, that is a clean and easy way of doing it too, gonna so some testing with different methods.
     
  9. @ValidateDev I'm afraid I don't have any 'official' code snippets to show to you, it's just a trick I often see and use in my code.
     
  10. I think the garbage collector doesn't work with static objects also, not sure on this but if your using static, its probably best to remember to set them to null after your finished with them in order to avoid memory leaks.

    Can anyone else better informed give an opinion on this?
     
  11. The GC doesn't recycle static fields as long as the class in which they're defined is loaded. In the context of a Java plugin it means the field won't be recycled until the plugin is disabled and unloaded.
     
  12. Why not simply load from a file and use a HashMap or various static fields that are set on initialization?