jzy3d-api
jzy3d-api copied to clipboard
Remove unused method parameters and replace Collection implementations by interfaces
Many methods in jzy3d have unused parameters that could be removed and some methods use implementations of Collections like ArrayList instead of the corresponding interface like List. While the first unnecessarily bloats the methods signature (with all the down sides), the second hinders to use other implementations of the same interface (e.g. a LinkedList) without need.
Therefore I suggest to remove unused method parameters and replace implementation types of parameters by the corresponding interface.
The worst I saw is org.jzy3d.plot3d.primitives.interactive.tools.ProjectUtils.project() that has two overrides one using a List and one an ArrayList as type for the second parameter. Instead I suggest to rename on of the overrides.
To find all occurrences of these API-flaws and to avoid such things in the future a static code analyzer like PMD/SpotBugs could be used during build (it checks the code during build and fails the build if violations against the configured rules are found) or like Sonar-Lint for the development IDE (Eclipse, IntelliJ IDEA, Visual Studio and VS-Code are supported).
The latter is something I can really recommend because it marks sub-optimal code immediately by underlining it blue and providing an explanation why a certain construct is bad.
Great suggestions. I fully agree with you on the List/ArrayList point.
I'll try Sonar Lint and tell you.
In the past I have also seen FindBugs as maven plugin.
Great.
I'm already using Sonar-Lint in my Eclipse (if you use Eclipse: you can get it from the market-place), so in can already tell you that there are many violations on some classes. Many of them are minor (for example you often use int arr[]; to declare an arrays variable, while the java convention respectively Sonar-Lint encourages to use int[] arr; (they explain in the SonarLint Rule Description View why they do). Besides such convention rules are are also many other handy rules to identify bad API or performance/security issues. Personally I agree to most of the rules, but if you don't like some they can be disabled. So in the beginning there are many violations but when they are fixed and you continue using it you will probably won't get new ones because they are never commited (at least this is the case for me).
Unfortunately integrating SonarLint into the build is not straight forward like for PMD/SpotBugs (it requires a separate sonar-cube server and does not simply report violations during the build). So I think it is not suitable for the build (but definitely good for the IDE). For the build I suggest to use PMD or SpotBugs.
The FindBugs project seems to be dead, but SpotBugs has stepped up to become its successor (I think some of the maintainers of FindBugs started SpotBugs when the project-lead of FindBugs abandoned the project.
From PMD I know that it can be set up to either just create reports about rules violations/errors or fail the entire build. The latter is handy because it prevents you from submitting code that violates the rules you selected.
Thanks. I installed it as Eclipse plugin and the results are interesting. There are however warnings that requires "moderation" so I do not think that adding PMD/SpotBugs to the builds is urgent (unless you are already used to include it in maven build and only need a few second to configure it).