In previous episodes, we showed some heuristics to find not so good code.
Code Smells
- Part I
- Part II
- Part III
- Part IV
- Part V
- Part VI
- Part VII
- Part VIII
- Part IX
- Part X
- Part XI
- Part XII
- Part XIII
- Part XIV
- Part XV
- Part XVI
- Part XVII
- Part XVIII
- Part XIX
- Part XX
Let’s fixed them!
Refactoring 001 - Remove Setters
Setters violate immutability and add accidental coupling
TL;DR: Make your attributes private to favor mutability
Problems Addressed
- Mutability
- setXXX() violates good naming policies since it does not exist on the MAPPER
- Accidental coupling
Related Code Smells
Steps
- Locate the setters' usage
- If you are setting essential properties move them to the constructor and remove the method
- If you need to change an accidental property it is not a setter. Remove the setXXX prefix
Sample Code
Before
public class Point {
protected int x;
protected int y;
public Point() {
this.x = 0;
this.y = 0;
}
public void setX(int x) {
this.x = x;
}
public void setY(int y) {
this.y = y;
}
}
Point location = new Point();
//At this momment, it is not clear which points represent
//It is coupled to constructor decision.
//Might be null or some other convention
location.setX(1);
//Now we have point(1,0)
location.setY(2);
//Now we have point(1,2)
public class Car {
protected int speed;
public Car() {
}
public void setSpeed(Speed desiredSpeed) {
this.speed = desiredSpeed;
}
}
Car tesla = new Car();
//We have no speed??
tesla.setSpeed(100 km/h);
//Now our car runs fast
After
//1. We locate setters usage
location.setX(1);
location.setY(2);
//2. If you are setting essential properties move
//them to the constructor and remove the method
public class Point {
public Point(int x, int y) {
this.x = x;
this.y = y;
//We remove the setters
}
Point location = new Point(1, 2);
public class Car {
protected int speed;
public Car() {
this.speed = 0 km/h;
}
public void speed(Speed desiredSpeed) {
this.speed = desiredSpeed;
}
}
//1. Locate the setters' usage
//3. If you need to change an accidental property
// it is not a setter. Remove the setXXX prefix
Car tesla = new Car();
//Our car is stopped
tesla.speed(100 km/h);
//We tell the desired speed. We don't set anything
//We don't care if the car stores its new speed.
//if it manages through the engine
//if the road is moving etc
Type
- [x]Semi-Automatic
We should detect setters (unless they use meta-programming) with our IDEs.
We can also remove them and see which tests fail if we have good coverage
Tags
- Mutability
Related Refactorings
- Remove Getters
- Pass essential properties in the constructor
- Initialize essential properties in the constructor
Credits
Refactoring 002 - Extract Method
Find some code snippets that can be grouped and called atomically.
TL;DR: Group your cohesive sentences together
Problems Addressed
- Readability
- Complexity
- Code Reuse
Related Code Smells
- Code Smell 03 - Functions Are Too Long
- Code Smell 05 - Comment Abusers
- Code Smell 18 - Static Functions
- Code Smell 22 - Helpers
- Code Smell 74 - Empty Lines
- Code Smell 78 - Callback Hell
- Code Smell 102 - Arrow Code
Steps
- Move the code fragment to a separate new method
- Replace the old code with a call to the recently created method.
Sample Code
Before
object Ingenuity {
fun moveFollowingPerseverance() {
// take Off
raiseTo(10 feet)
// move forward to perseverance
while (distanceToPerseverance() < 5 feet) {
moveForward()
}
// land
raiseTo(0 feet)
}
After
object Ingenuity {
//1. Move the code fragment to a separate new method
private fun takeOff() {
raiseTo(10 feet)
}
//1. Move the code fragment to a separate new method
private fun moveForwardToPerseverance() {
while (distanceToPerseverance() < 5 feet) {
moveForward()
}
}
//1. Move the code fragment to a separate new method
private fun land() {
raiseTo(0 feet)
}
fun moveFollowingPerseverance() {
takeOff()
//2. Replace the old code with a call to the recently created method.
moveForwardToPerseverance()
//2. Replace the old code with a call to the recently created method.
land()
//2. Replace the old code with a call to the recently created method.
}
}
Type
- [x]Automatic
Many IDEs support this safe refactoring
Why code is better?
Code is more compact and easier to read.
Functions can be reused.
Algorithms and functions are more declarative hiding implementative details on extracted code.
Limitations
Does not work well if you use meta-programming anti-pattern.
Tags
- Complexity
- Readability
Related Refactorings
- Move method to a new class
Credits
Refactoring 003 - Extract Constant
You need to use some values explaining their meaning and origin
TL;DR: Name all your magic numbers
Problems Addressed
- Readability
- Complexity
- Code Reuse
Related Code Smells
Steps
- Move the constant code fragment to a constant declaration
- Replace the values with a reference to the constant.
Sample Code
Before
double energy(double mass) {
return mass * 300.000 ^ 2;
}
After
//1. Move the constant code fragment to a constant declaration
final double LIGHT_SPEED = 300.000;
double energy(double mass) {
//2. Replace the old code with a reference to the constant.
return mass * LIGHT_SPEED ^ 2;
}
Type
- [x]Automatic
Many IDEs support this safe refactoring
Why code is better?
Constant names add meaning to our code.
Magic numbers are difficult to understand and change.
Code must be as declarative as possible.
Tags
- Readability
Related Refactorings
Refactoring 002 - Extract Method
Credits
Refactoring 004 - Remove Unhandled Exceptions
Creating YAGNI exception classes pollutes our environment. Let's remove them.
TL;DR: Remove unnecessary and not references empty exception classes.
Problems Addressed
- Empty Classes
- Namespace Polluted
Related Code Smells
Code Smell 26 - Exceptions Polluting
Steps
- Check there are no references to the empty exception class.
- Replace the throw sentence with a generic one.
Sample Code
Before
class RangeNotSatisfiedException < StandardError
end
begin
raise RangeNotSatisfiedException.new "Range must be betweet 0 and 10"
rescue RangeNotSatisfiedException => e
puts e.message
puts e.exception_type
end
After
# 1. Check there are no references to the empty exception class.
# 2. Replace the throw sentence with a generic one.
begin
raise StandardError.new "Range must be betweet 0 and 10"
rescue StandardError => exception
puts exception.message
puts exception.exception_type
end
Type
- [x]Automatic
If the Exception class has no references we can perform a Safe Remove and replace it with Exception class.
Why the code is better?
- We remove an empty class nobody uses.
- We shrink the code
Limitations
If we need to declare an empty exception class as documentation for an API module, our clients might need to catch it.
This is a gold plating and YAGNI example.
Tags
- Clean up
Related Refactorings
- Safe Remove
Credits
Image by danielkirsch on Pixabay
Refactoring 005 - Replace Comment with Function Name
Comments should add value. And function names too.
TL;DR: Don't comment on what you are doing. Name what you are doing.
Problems Addressed
- Bad Names
- Comments
Related Code Smells
Code Smell 05 - Comment Abusers
Code Smell 75 - Comments Inside a Method
Code Smell 06 - Too Clever Programmer
Steps
- Name the function with the previous comment
- Remove the Comment
Sample Code
Before
<?
function repl($str) {
// Replaces with spaces the braces
$str = str_replace(array("\{","\}")," ",$str);
return $str;
}
After
<?
// 1. Name the function with the previous comment
// 2. Remove the Comment
function replaceBracesWithSpaces($input) {
return str_replace(array("\{","\}")," ", $input);
}
Type
- [x]Semi-Automatic
Some IDEs have this refactoring although naming is not fully automatic.
Why the code is better?
Comments always lie.
It is hard to maintain comments.
On the contrary, Functions are alive and self-explanatory.
Limitations
As always, very important design decisions are valid comments.
Tags
- Comments
See also
Credits
Image by Jannik Texler on Pixabay
This article is part of the Refactoring Series.
More smells and more refactorings are on the way!