In previous episodes, we showed some heuristics to find not so good code.
Let’s fixed them!
Setters violate immutability and add accidental coupling
TL;DR: Make your attributes private to favor mutability
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
//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
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
Find some code snippets that can be grouped and called atomically.
TL;DR: Group your cohesive sentences together
object Ingenuity {
fun moveFollowingPerseverance() {
// take Off
raiseTo(10 feet)
// move forward to perseverance
while (distanceToPerseverance() < 5 feet) {
moveForward()
}
// land
raiseTo(0 feet)
}
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.
}
}
Many IDEs support this safe refactoring
Code is more compact and easier to read.
Functions can be reused.
Algorithms and functions are more declarative hiding implementative details on extracted code.
Does not work well if you use meta-programming anti-pattern.
You need to use some values explaining their meaning and origin
TL;DR: Name all your magic numbers
double energy(double mass) {
return mass * 300.000 ^ 2;
}
//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;
}
Many IDEs support this safe refactoring
Constant names add meaning to our code.
Magic numbers are difficult to understand and change.
Code must be as declarative as possible.
Refactoring 002 - Extract Method
Creating YAGNI exception classes pollutes our environment. Let's remove them.
TL;DR: Remove unnecessary and not references empty exception classes.
Code Smell 26 - Exceptions Polluting
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
# 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
If the Exception class has no references we can perform a Safe Remove and replace it with Exception class.
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.
Image by danielkirsch on Pixabay
Comments should add value. And function names too.
TL;DR: Don't comment on what you are doing. Name what you are doing.
Code Smell 05 - Comment Abusers
Code Smell 75 - Comments Inside a Method
Code Smell 06 - Too Clever Programmer
<?
function repl($str) {
// Replaces with spaces the braces
$str = str_replace(array("\{","\}")," ",$str);
return $str;
}
<?
// 1. Name the function with the previous comment
// 2. Remove the Comment
function replaceBracesWithSpaces($input) {
return str_replace(array("\{","\}")," ", $input);
}
Some IDEs have this refactoring although naming is not fully automatic.
Comments always lie.
It is hard to maintain comments.
On the contrary, Functions are alive and self-explanatory.
As always, very important design decisions are valid comments.
Image by Jannik Texler on Pixabay
This article is part of the Refactoring Series.
More smells and more refactorings are on the way!