Refactoring required in the code
amansbhandari opened this issue · 0 comments
-
Rename Method/Variable
1.1. Location The source file can be found at path /metrics/metrics-json/src/main/java/com/codahale/metrics/json/MetricsModule.java with below details.
Package name : com.codhale.metrics.json
Class name : MetricsModule.java
Method name : calculateRateUnit()
Line number : 258
Link of the file before refactoring : https://github.com/dropwizard/metrics/blob/release/4.2.x/metrics-json/src/main/java/com/codahale/metrics/json/MetricsModule.java
Link of the file after refactoring :
https://github.com/amansbhandari/metrics/blob/release/4.2.x/metrics-json/src/main/java/com/codahale/metrics/json/MetricsModule.java
1.2. Why refactor this?
With the variable name s it was harder to understand what exactly variable s is storing. By changing the name to timeUnitInUSLocale reflects more readability that it will be carrying the unit of time in US locale.
-
Extract Method II
2.1. Location The source file can be found at path /metrics/metrics-jakarta-servlets/src/main/java/io/dropwizard/metrics/servlets/AdminServlet.java
Package name : io.dropwizard.metrics.servlets
Class name : AdminServlet.java
Methods name : service(), callServletService()[new extracted method]
Line number : 148
Link of the file before refactoring : https://github.com/dropwizard/metrics/blob/release/4.2.x/metrics-jakarta-servlets/src/main/java/io/dropwizard/metrics/servlets/AdminServlet.java
Link of the file after refactoring : https://github.com/amansbhandari/metrics/blob/release/4.2.x/metrics-jakarta-servlets/src/main/java/io/dropwizard/metrics/servlets/AdminServlet.java
2.2. Why refactor this?
service() function is basically checking if the service is enabled and is then calling 5 services one by one. One catch here is that these 5 services are 5 different classes but implements one singe interface – HttpServlet. So extracted one method that will be common for all 5 services and since their parent is a single interface, accessed the inherited function from the superclass reference. (check new method : callServletService())
-
Change bidirectional association to unidirectional association
3.1. Location The source file can be found at path /metrics/metrics-core/src/main/java/com/codahale/metrics/Clock.java
Package name : com.codahale.metrics;
Class name : Clock.java
Line number : 36
Link of the file before refactoring : https://github.com/dropwizard/metrics/blob/release/4.2.x/metrics-core/src/main/java/com/codahale/metrics/Clock.java
Link of the files after refactoring : https://github.com/amansbhandari/metrics/blob/release/4.2.x/metrics-core/src/main/java/com/codahale/metrics/Clock.java https://github.com/amansbhandari/metrics/blob/release/4.2.x/metrics-core/src/main/java/com/codahale/metrics/UserTimeClock.java https://github.com/amansbhandari/metrics/blob/release/4.2.x/metrics-core/src/main/java/com/codahale/metrics/UserTimeClockHolder.java
New files added: UserTimeClock.java, UserTimeClockHolder.java
3.2. Why and how refactored this?
The UserTimeClock class is defined as a static variable inside the Clock class. UserTimeClock is extending Clock class resulting it into cyclic-dependent smell between these two classes. One more point to note here is Clock class is not at all using its UserTimeClock class and hence we can change this bidirectional cyclic association to one directional association.
So I moved the UserTimeClock class outside clock class in a separate .java file and removed the static variable dependency. Now UserTimeClock is free class and Clock is not dependent on this class anymore. But the UserTimeClock is still dependent on Clock as a subclass.
-
Extract Class
4.1. Location The source file can be found at path metrics/metrics-core/src/main/java/com/codahale/metrics/MetricsRegistry.java
Package name : com.codahale.metrics;
Class name : MetricRegistry.java
Line number : 21
Link of the file before refactoring : https://github.com/dropwizard/metrics/blob/release/4.2.x/metrics-core/src/main/java/com/codahale/metrics/MetricRegistry.java
Link of the file after refactoring : https://github.com/amansbhandari/metrics/blob/release/4.2.x/metrics-core/src/main/java/com/codahale/metrics/MetricRegistry.java
https://github.com/amansbhandari/metrics/blob/release/4.2.x/metrics-core/src/main/java/com/codahale/metrics/NameUtility.java
New file added: NameUtility.java
4.2. Why and how refactored this?
The MetricsRegistry class has LCOM of 0.82 and hence contains MultiFaceted Abstraction that means it has cohesion between methods is low. Keeping the same in mind, methods name(), name(Class<?>…) and append() are static methods used to build a dotted name and can be extracted into a new class. After extracting it to a new class, all the callers needed to be refactored accordingly. For the same, I used the IntelliJ extract/introduce refactor feature.
-
Move Method
5.1. Location The source file can be found at path metrics/metrics-core/src/main/java/com/codahale/metrics/MetricsRegistry.java
Package name : com.codahale.metrics;
Class name : MetricRegistry.java
Line number : 21
Link of the file after refactoring : https://github.com/amansbhandari/metrics/blob/release/4.2.x/metrics-core/src/main/java/com/codahale/metrics/MetricRegistry.java
Functions moved: NameUtility.name(), NameUtility.append(), NameUtility.name(..)
5.2. Why and how refactored this?
MetricsRegistry class has low cohesion and hence moved three functions named name(), name(..) and append() to another class called NameUtility(). These three methods have high cohesion with each other and is better suited in NameUtility() class.