Auburn / FastNoiseLite

Fast Portable Noise Library - C# C++ C Java HLSL GLSL JavaScript Rust Go
http://auburn.github.io/FastNoiseLite/
MIT License
2.79k stars 327 forks source link

Compliant java code #58

Closed gamerover98 closed 3 years ago

gamerover98 commented 3 years ago

Adapted the old java code to the newest java compliant code solutions with IDE and Sonar (Code quality checker) tools.

Solved issues

Unresolved issues

Auburn commented 3 years ago

Hi gamerover, Could you expand what changed related to:

Fixed some code smells, removed useless switch brackets and useless (duplicated) variables. Added some warning suppression to unused methods

It's hard to see in a diff since everything has changed. Thanks.

gamerover98 commented 3 years ago

Hi Auburn, I'll explain these things:

Added some warning suppression to unused methods

At the top of the FastNoiseLite class, I've added a SuppressWarnings annotation with the unused argument. it disables the IDE "unused method" warning. It will disappear while compiling, it is just for a syntactic purpose.

@SuppressWarnings("unused")
public class FastNoiseLite {
...
}

There are a lot of SuppressWarnings annotations inside the code to prevent the same syntactic things inside methods.

// SIMPLEX/OPEN_SIMPLEX_2 Noise
@SuppressWarnings("squid:S125") // this block contains commented-out lines of code that should be removed
private float singleSimplex(int seed, /*FNLfloat*/ float x, /*FNLfloat*/ float y) {
 ....
}

The squid is the SonarQube issue ID and you can find more info here

If you need to deactivate a rule (or all rules) for an entire file, then issue exclusions are the way to go. But if you only want to deactivate a rule across a subset of a file - all the lines of a method or a class - you can use @SuppressWarnings("all") or @SuppressWarnings with rule keys: @SuppressWarnings("squid:S2078") or @SuppressWarnings({"squid:S2078", "squid:S2076"}).

Where S125 (or S2078) is the issue number :)

relevant commits: eec040c and 8a27939

Fixed some code smells, removed useless switch brackets and useless (duplicated) variables

I forgot to tell you that these code improvements are for java 8+

Auburn commented 3 years ago

Hi Gamerover, After looking through this and talking it over with some other people I don't think we want to take this change. Most of these changes are just for the sake of following a strict code format policy. Currently it's possible to compare FastNoise Lite between the various language versions and get a reasonable diff, this is very helpful when making changes and trying to reflect them across every language. In certain cases the changes make the code less readable, things like the commented out code would be very useful for people using this code as a reference, it has been left in on purpose.

Thank you for the time taken to do this, but I don't think it is the right decision to merge this.