Don't know if this is the right subforum, or if there are any rules against this... But could anyone go through the source code below, and tell if this can be called "good code", or if there are beginner mistakes etc. within? (I'm unsure at this point.) https://github.com/StjernholmPW/DelayedSpawners This is my first public spigotmc.org resource, just want a push forward, and some improvement points I could work on. Thank you. Stjernholm
Well, being a Java noob I'm not really qualified to judge, but here are my thoughts: In the primary class, those enable/disable messages are pointless log clutter. Spigot already sends them. In the command class, the switch should be on args[0].toLowerCase() to prevent case issues and using returns would cut down on indentation. Not that it's a bad thing, just a matter of personal preference. Event classes do not use proper naming convention In EditSpawnerMenu, it would've been easier to use a collection instead of a whole bunch of switch cases I see from the config mgr you apparently aren't aware that the config get options allow for defaults, so instead of like Code (Text): public int getMinSpawnDelay(String entity) { if(valueExist(entity, "mindelay")) { return delayedSpawners.getConfig().getInt("type." + entity + ".mindelay"); } return 200; } You could just do Code (Text): public int getMinSpawnDelay(String entity) { return delayedSpawners.getConfig().getInt("type." + entity + ".mindelay", 200); } Looks reasonable to me overall.
not that important but packages should be all lowercase and in your pom you dont need the maven shade plugin since you dont shade any dependencies
The way you're implementing InventoryHolder is discourage: You can just keep track of your custom Inventory instances in a HashSet and check if the inventory in question is within said set.
Just to clarify on this, does this mean that one should not create their own class implementing the InventoryHolder interface, and then setting their custom inventory's holder as their custom class, with the goal to later be able to check if an inventory is a custom inventory by using instanceof? This is only semi-relevant to OP, sorry for that, I'm asking for personal curiosity. Also, for OP: You could try using the final statement, it's pretty simple once you get a hang of it, it makes your code more readable and gives a small performance boost.
Also comments are good helping hands for your future improvements. You never know when will you open your project again to add some functions or fix bugs. In this small project it isn't as mandatory as in huge projects but it still improves readability of the code.