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
Code Smell 109 - Automatic Properties
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, then 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 moment, it is not clear which points represent
// It is coupled to the 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 fastAfter
// 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 etcType
[X] Semi-Automatic
You should detect setters (unless they use meta-programming) with your IDEs.
You can also remove them and see which tests fail if you have good coverage.
Safety
This is not a safe refactoring since you might miss some methods calling the removed setters.
You need to make sure to have good coverage and also an excellent QA process to ensure a smooth refactoring.
Why is the Code Better?
This refactoring improves encapsulation and integrity adding an extra access control layer.
Refactor with AI
Without Proper Instructions
With Specific Instructions
Tags
Mutability
Related Refactorings
Refactoring 016 - Build With The Essence
Remove Getters
Credits
This article is part of the Refactoring Series