Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactoring Code #354

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

krishnamodi2000
Copy link

Original Commit from which the branchwas forked and cloned: e2d6788

Set I:

Refactoring name: Extract Method Refactoring
Location :
Package: org.jfree.chart.axis
Class: CyclicNumberAxis
Method: valueToJava2D
Line Number: 767
Link of the files(s) of the previous commit :e2d6788d594c51941ddae337ae666fda5c52ad9f
Link of the files(s) of the commit with refactoring changes : 4ccdc7e

To reduce the if else statements and the complexity of the method we extracted two methods in the class from the conditions. One was valueToJava2DInverted and other was valueToJava2DNormal. These two methods contained only the required logic and further by calling them in the value2DJava method it made the method more readable, understandable and easily testable.

Refactoring name: Decompose Conditional Refactoring
Location :
Package: org.jfree.chart.axis
Class: LogarthmicAxis
Method: refreshTicksHorizontal
Line Number: 718
Link of the files(s) of the previous commit :4ccdc7eed0a114273765cd18e551932e1f5126a3
Link of the files(s) of the commit with refactoring changes : 0a246e1

Here we changed the complex conditional in the if to a separate method that is directly called in the if. This makes the code more readable and we can understand what exactly are the functions and comparisons are happening in the if statement are understandable. We have done this by creating the method shouldShowTickLabel.

Refactoring name: Renaming Variable Refactoring
Location :
Package: org.jfree.chart.axis
Class: CategoryAxis
Method:calculateCategoryLabelWidth
Line Number: 1210
Link of the files(s) of the previous commit :0a246e137e4ba1c366120a5e0f4d689efc0588df
Link of the files(s) of the commit with refactoring changes : b48969f
Here we replace double w with double width. This is done so that the code as the name of a variable plays important role in describing the behavior to the developer. Unclear, meaningless or confusing names make the things hard to understand

Set II:

Refactoring name: Replace conditional with polymorphism Refactoring
Location:
Package: org.jfree.chart.legend
Class: LegendGraphic| Method:arrange
Line Number: 482
Link of the files(s) of the previous commit : b48969f
Link of the files(s) of the commit with refactoring changes:5b497b144bc26d2e4c2469060132e5cb6f8be40

The code was refactored using the Replace Conditional with Polymorphism technique. This involves creating separate classes for each case of the switch statement and moving the logic for each case into their respective classes. A new interface or abstract class for the different width constraint types was created. Separate classes for each width constraint type, implementing the WidthConstraint interface were created. The switch statement were replaced with a call to the appropriate width constraint class. This way, each width constraint type has its own class and logic, making it easier to maintain and extend in the future. The switch statement is replaced with polymorphism, allowing for more flexibility and scalability in the code.

Refactoring name: Pull Up variable Refactoring
Location:
Package: org.jfree.chart.annotation
Class:AbstractAnnotation
Line Number: 61 in AbstractAnnotation
Line Pulled Up In the Class: public static final Paint DEFAULT_PAINT = Color.Black
Pulled From Classes:TextAnnotation, XYTextAnnotation
Line Number: 69 in TextAnnotation, Usage in Line Number:109
Line Number:81 in XYTextAnnotation, Usage in Line Number:153
Link of the files(s) of the previous commit : 5b497b1
Link of the files(s) of the commit with refactoring changes: 7ce4584

Here we have pulled up the public static final Paint DEFAULT_PAINT = Color.Black from two classes where it was first implemented. This is because it is a common behaviour in the fields. It allowed us to capture similarities among siblings and promote to super type. This also removed duplication of statements.

Refactoring name: Extract Class Refactoring
Location:
Package: org.jfree.chart.swing
Class: PolarChartPanel
Line Number: 125
Extracted Class: PolarChartPopupMenu
Link of the files(s) of the previous commit : 7ce4584
Link of the files(s) of the commit with refactoring changes: fe52e1b

Here we have created another class for creating the popup menu of PolarChartPanel. This is because each abstraction should have a unique responsibility. In case, an abstraction is changed for multiple causes then it is a multifaceted abstraction (having multiple responsibilities). Here the PloarChartPanel had a lot of responsibilities. We have separated one such responsibility from it.

…ss: CyclicNumberAxis| Method: valueToJava2D| Line Number: 767
…is| Class: LogarthmicAxis| Method: refreshTicksHorizontal| Line Number: 718
…Class: CategoryAxis| Method:calculateCategoryLabelWidth | Line Number: 1210
…rg.jfree.chart.legend| Class: LegendGraphic| Method:arrange | Line Number: 482
…tion| Class:AbstractAnnotation | Line Pulled Up In the Class: public static final Paint DEFAULT_PAINT = Color.Black |Pulled From Classes:TextAnnotation, XYTextAnnotation| Line Number: 69 in TextAnnotation, Usage in Line Number:109| Line Number:81 in XYTextAnnotation, Usage in Line Number:153| Line Number: 61 in AbstractAnnotation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant