Critique my code
- Thread starter I_Luv_Cowz
- Start date
PebbleHost
High performance, consistent uptime and fast support. Minecraft hosting that just works.
I'd start by telling you that you shouldn't name your main package io.github, because if you have multiple plugins in the same test server, those will interact with eachother and won't work properly. You should name it io.github.(plugin name) or something like this.
I'm on my phone right now, it's hard to quote from the code, so when I get on my PC I will update my answer.
I'm on my phone right now, it's hard to quote from the code, so when I get on my PC I will update my answer.
First off, I'd recommend switching from Maven to Gradle as it's way easier to use, way easier to read and overall more cleaner. Besides that, most huge companies, such as Google, Amazon etc. are all using Gradle.
Afterwards, I suggest you quit using Lombok, in my opinion it's a really bad tool that will just make you lazier, when in fact, there's a ton of risks associated with it, read more here. Instead, you could integrate some of your code with Kotlin or Groovy if you want to type less.
Your commands are a mess, if you wanna keep using them as you're currently doing, utilise a switch statement, as if you have 2.000 lines of if/elses, the program will check every block from top to bottom until reaching your desired block of code. This can lead to huge performance issues overtime. A better alternative to commands is definitely Aikar's commands - it's a very cool library which allows you to easily create commands and directly having them inserted into Bukkit's command map.
In your code, there's also a lot of chunks where you're always checking for a non-negated true condition - by that I mean:
This can easily become unreadable. An easy fix is the following:
Both codes do the exact same thing, but the second version is easier to read and understand.
This is definitely not how you want to use the builder pattern.
It's good that your HTTPRequest and HTTPResponse operations are ran asynchronously, but I'd suggest taking a look at CompletableFuture & ExecutorService instead of using Bukkit's asynchronous worker. The main difference would be the addition of CompletionStages to your code, making it far more easier to understand & cleaner.
Another problem that I'm seeing in your code is again, something that's more of a design flaw. Methods that throw Exceptions shouldn't have a try-catch block inside of them. Instead, add the throws keyword, followed by the Exception they throw to them, and handle the Exception when you're using the method outside of it's root declaration.
Also, don't use
Also, take a look at JetBrains' Annotations, probably one of the most useful tools out there.
Also, the class extending JavaPlugin should never be a mess, utilise OOP principles to get rid of messy code. Take a look at a "pseudo-main" class I wrote for a 200 classes-ish plugin half an year ago.
Besides that, most of it is fine for the size of the plugin you've made, but I can't comment any more on it since the plugin is quite small in size. Good luck learning!
Afterwards, I suggest you quit using Lombok, in my opinion it's a really bad tool that will just make you lazier, when in fact, there's a ton of risks associated with it, read more here. Instead, you could integrate some of your code with Kotlin or Groovy if you want to type less.
Your commands are a mess, if you wanna keep using them as you're currently doing, utilise a switch statement, as if you have 2.000 lines of if/elses, the program will check every block from top to bottom until reaching your desired block of code. This can lead to huge performance issues overtime. A better alternative to commands is definitely Aikar's commands - it's a very cool library which allows you to easily create commands and directly having them inserted into Bukkit's command map.
In your code, there's also a lot of chunks where you're always checking for a non-negated true condition - by that I mean:
Code:
if (condition) {
// code
if (anotherCondition) {
// code
}
}This can easily become unreadable. An easy fix is the following:
Code:
if (!condition) return;
// code
if (!anotherCondition) return;
// codeBoth codes do the exact same thing, but the second version is easier to read and understand.
Code:
ItemBuilder builder = store.getItemFromMaterialString(player, materialString);
builder.amount(amount);
builder.coloredName(PlaceholderUtil.parsePapiPlaceholders(player, display));
builder.coloredLore(PlaceholderUtil.parsePapiPlaceholders(player, lore));
ItemStack item = builder.create();This is definitely not how you want to use the builder pattern.
It's good that your HTTPRequest and HTTPResponse operations are ran asynchronously, but I'd suggest taking a look at CompletableFuture & ExecutorService instead of using Bukkit's asynchronous worker. The main difference would be the addition of CompletionStages to your code, making it far more easier to understand & cleaner.
Another problem that I'm seeing in your code is again, something that's more of a design flaw. Methods that throw Exceptions shouldn't have a try-catch block inside of them. Instead, add the throws keyword, followed by the Exception they throw to them, and handle the Exception when you're using the method outside of it's root declaration.
Also, don't use
Code:
new Thread(runnable).start();
// instead use
CompletableFuture.runAsync(runnable);Also, take a look at JetBrains' Annotations, probably one of the most useful tools out there.
Also, the class extending JavaPlugin should never be a mess, utilise OOP principles to get rid of messy code. Take a look at a "pseudo-main" class I wrote for a 200 classes-ish plugin half an year ago.
Besides that, most of it is fine for the size of the plugin you've made, but I can't comment any more on it since the plugin is quite small in size. Good luck learning!
Last edited:
