hageldave / JPlotter

OpenGL based 2D Plotting Library for Java using AWT and LWJGL
https://github.com/hageldave/JPlotter/wiki
MIT License
45 stars 6 forks source link

refactorv1 #37

Closed Ch1rag02 closed 2 years ago

Ch1rag02 commented 2 years ago

[Modify] [1] Rename variable:
In file JplotterCanvas.java there was an element object named defs, this naming convention is not meaningful and doesn't provides the clarity to the developer hence, I have changed the name and it's references to elementOfDocument. Secondly, WindowListener object was declared as l now it is changed to listener.

[2] Extract Method: To remove the complex statements and make the code readable I have extracted 4 methods from Shader.java namely vertexShaderID(), geometryShaderID(), fragmentShaderID() and shaderProgID()

[3] Pull up method: There were few child classes that were using the same method so, I pulled-up those method in SuperClass from TriangleRenderer, CurveRenderer, TextRenderer, LinesRenderer, PointsRenderer as they were not having any specific implementation of their own.

[4] Extract Class: Extracted class LabelsForTicks from ExtendedWilkinson.

[5] Move Method: Moved 4 method namely getTexCoordXForCharLeft, getTexCoordXForCharRight, getTexCoordYForCharTop, getTexCoordYForCharBot from CharacterAtlas.java Class as the usage of the method was done in SignedDistanceCharacters.java Class.

hageldave commented 2 years ago

First off: you are offering a PR with exactly how many changed/new files? Screenshot from 2022-03-27 01-37-14

Next up: you are changing the names of in-method variables to fit your personal taste. Moreover, changing a variables name from defs to a very generic elementOfDocument clearly removes meaning.

Next: you are extracting a method and name it vertexShaderID(src), what a great name for what the responsibility of the method is. Screenshot from 2022-03-27 01-52-24 Also there are no complex statements as you put it, only lots of incomprehension.

Also very useful: moving a static method to a new class and naming the class according to the method.

All of the changes are highly controversial and do not add value to the project. This PR is beyond useless.