CuukyOfficial / VaroPlugin

Ein sehr funktionsreiches und performantes ODV & Varo Plugin mit API u.v.m.
https://docs.varoplugin.de
GNU Affero General Public License v3.0
17 stars 21 forks source link

Improve updater #24

Closed JeromeWolff closed 4 years ago

JeromeWolff commented 4 years ago

Improve updater by adding "try-with-resources", missing braces and lambda expressions. I also removed unnecessary assignments.

JeromeWolff commented 4 years ago
  1. Once a variable has been initialized, it can no longer be changed. Since they are not overwritten anywhere, the highest priority is to create clarity.
  2. To improve clarity and prevent bugs like Apple's goto bug.
  3. What line are you talking about?
CuukyOfficial commented 4 years ago
  1. Yeah I'm familiar with the use of the final modifiers, but I only use it if there is a special reason why it shouldn't be changed (for example in calculations or constants), otherwise I just let the compiler do its job.
  2. I've never made bugs like this before, if you format your code right you won't make that kind of mistakes
  3. VaroUpdateResultSet:23
JeromeWolff commented 4 years ago

You can do that, yes. However, it provides a nicer structure just like partitioning variables of the same type. To point number 2: You can accidentally produce bugs with it, but it is not a must. However, any experienced developer will tell you that this application is much clearer and more beautiful. Writing code quickly is one thing, good and beautiful code is another.

CuukyOfficial commented 4 years ago

I don't really agree with the code style changes, the lambda change and the "try-with-resources" is good tho. If you could change that I'd accept the pull request.

JeromeWolff commented 4 years ago

The revised memory model proposed by JSR 133 includes special provisions for final fields, provisions that are absent from the existing specification. Newer VMs already implement this specification, and treat final fields accordingly. Because final fields are assigned exactly once, aggressive optimizations in a multithreaded context become possible. Specifically, a field doesn't need to be ever reloaded, since its value is guaranteed never to change.

CuukyOfficial commented 4 years ago

Okay maybe that's a good point, I will change that in a later recode of this project. Still, the brackets and the declarations are still a problem for me

JeromeWolff commented 4 years ago

As I have already said, the braces will not produce errors from future developers.

CuukyOfficial commented 4 years ago

I prefer not using the braces in such cases, it looks nicer - it's personal preference. Same thing with the declaration

JeromeWolff commented 4 years ago

Does it look better now?

CuukyOfficial commented 4 years ago

That's more like it. But why did you remove the private modifiers in the enums?

JeromeWolff commented 4 years ago

No matter which modifier a constructor of an enum has, it can only be called within the class. Therefore you always take package-private.

CuukyOfficial commented 4 years ago

Yeah I know that, but I want to have a modifier in front of a constructor - personal preference

JeromeWolff commented 4 years ago

Why do you use so many unnecessary things?

JeromeWolff commented 4 years ago

The code is very inefficient, there are memory leaks, performance problems and it is mostly hardcoded. And why do you have something like that in your code:

if (configurations.containsKey(section.getFolder() + "/" + section.getName())) {
        System.out.println(Main.getConsolePrefix() + "PAUL NEIN");
    Bukkit.getServer().shutdown();
}
CuukyOfficial commented 4 years ago

It's a joke - a contributor once made a mistake which cost me hours of fixing, so I made this. And if you think that this code is performing badly, why don't you prove that to me instead of just dropping random statements

CuukyOfficial commented 4 years ago

And there are many things in this code that I don't like anymore (like a bad structure, final etc). But until I didn't recode it, I won't change things in order to keep things simple and similar.

JeromeWolff commented 4 years ago

You use multiple nested loops, which are executed synchronously. The same is true for MySQL: there you use statements instead of prepared statements, which are faster and more secure against SQL injections. Also MySQL is never executed asynchronously. With a high probability you have several entries in the table from one player.

CuukyOfficial commented 4 years ago

I didn't knew I don't use prepared statements, I thought I would. Still, this won't make any noticeable difference in performance, but I will change that. And the main game loop for example has to be synchronized - Bukkit doesn't like handling the players asynchronously and even that doesn't cost any performance.

JeromeWolff commented 4 years ago

When you execute something synchronously, you wait for it to finish before moving on to another task. When you execute something asynchronously, you can move on to another task before it finishes.