ℹ️ This repository is part of my Refactoring catalog based on Fowler's book with the same title. Please see kaiosilveira/refactoring for more details.
| Before | After |
|---|---|
targetTemperature(aPlan);
function targetTemperature(aPlan) {
currentTemperature = thermostat.currentTemperature;
// rest of the function |
targetTemperature(aPlan, thermostat.currentTemperature);
function targetTemperature(aPlan, currentTemperature) {
// rest of the function |
Inverse of: Replace Parameter with Query
External dependencies are easy to use, highly available with minimal overhead, and often require no initialization, but their downsides often outweight all these positive attributes: global shared states can become complicated to manage as the codebase grows, it's harder to track side effects throughout the system, and testing the code that depend on them can become a nightmare. This refactoring sheds some light on how to move away of these cases.
Our working example is a program that manages the temperature of a thermostat, given a HeatingPlan. The TemperatureManager itself can be seen as the following set of functions:
export function setToHeat() {
thermostat.mode = 'heat';
}
export function setToCool() {
thermostat.mode = 'cool';
}
export function setOff() {
thermostat.mode = 'off';
}
export function handleThermostatReading(thePlan, thermostat) {
if (thePlan.targetTemperature > thermostat.currentTemperature) setToHeat();
else if (thePlan.targetTemperature < thermostat.currentTemperature) setToCool();
else setOff();
}And the HeatingPlan looks like this:
export class HeatingPlan {
constructor({ min, max }) {
this._min = min;
this._max = max;
}
get targetTemperature() {
if (thermostat.selectedTemperature > this._max) return this._max;
else if (thermostat.selectedTemperature < this._min) return this._min;
else return thermostat.selectedTemperature;
}
}And both of the above modules depend on a global thermostat object:
export let thermostat = {
currentTemperature: 70,
selectedTemperature: 72,
};We know for enough time now that depending on globals is a bad idea, so our plan here is to remove the dependency HeatingPlan has on thermostat.
HeatingPlan's test suite covers returning the minimum or maximum target temperature within its predefined range if the underlying thermostat temperature is outside of the boundaries, and also returning the thermostat's temperature itself if it is within the range.
TemperatureManager's test suite covers updating the thermostat mode to either heat, cool, or off, depending on the circumstances.
The implementation of these tests is extensive, so they'll not be shown here. You can always go ahead and check heating-plan/index.test.js and temperature-manager/index.test.js for details.
As stated above, we want to get rid of the dependency HeatingPlan has on the global thermostat object. To do so, we need to receive its values as arguments, instead of directly referencing them. The strategy, then, is to turn the targetTemperature getter into a function, so we can receive these arguments and be free. To do so, we first need to limit the spread of thermostat inside targetTemperature. We start by extracting a selectedTemperature variable:
diff --git HeatingPlan
get targetTemperature() {
- if (thermostat.selectedTemperature > this._max) return this._max;
- else if (thermostat.selectedTemperature < this._min) return this._min;
- else return thermostat.selectedTemperature;
+ const selectedTemperature = thermostat.selectedTemperature;
+ if (selectedTemperature > this._max) return this._max;
+ else if (selectedTemperature < this._min) return this._min;
+ else return selectedTemperature;
}Then, we can move forward with introducing our envisioned function, naming it with an easily searcheable prefix:
diff --git HeatingPlan
+ xxNEWtargetTemperature(selectedTemperature) {
+ if (selectedTemperature > this._max) return this._max;
+ else if (selectedTemperature < this._min) return this._min;
+ else return selectedTemperature;
+ }We can then start using xxNEWtargetTemperature in the body of the targetTemperature getter:
diff --git HeatingPlan
get targetTemperature() {
const selectedTemperature = thermostat.selectedTemperature;
- if (selectedTemperature > this._max) return this._max;
- else if (selectedTemperature < this._min) return this._min;
- else return selectedTemperature;
+ return this.xxNEWtargetTemperature(selectedTemperature);
}Since almost all the code has now been moved to a separate function, we don't need the selectedTemperature temp anymore, so we inline it:
diff --git HeatingPlan
get targetTemperature() {
- const selectedTemperature = thermostat.selectedTemperature;
- return this.xxNEWtargetTemperature(selectedTemperature);
+ return this.xxNEWtargetTemperature(thermostat.selectedTemperature);
}And now that targetTemperature is simple enough, we can just inline it in the calling code:
diff --git TemperatureManager
export function handleThermostatReading(thePlan, thermostat) {
- if (thePlan.targetTemperature > thermostat.currentTemperature) setToHeat();
- else if (thePlan.targetTemperature < thermostat.currentTemperature) setToCool();
+ if (thePlan.xxNEWtargetTemperature(thermostat.selectedTemperature) > thermostat.currentTemperature) setToHeat();
+ else if (thePlan.xxNEWtargetTemperature(thermostat.selectedTemperature) < thermostat.currentTemperature) setToCool();
else setOff();
}With this move, the targetTemperature getter is now unused, so we can remove it from HeatingPlan:
diff --git HeatingPlan
- get targetTemperature() {
- return this.xxNEWtargetTemperature(thermostat.selectedTemperature);
- }And now we can reach our goal: HeatingPlan does not depend on thermostat any longer:
diff --git HeatingPlan
-import { thermostat } from '../thermostat.js';
-
export class HeatingPlan {
constructor({ min, max }) {
this._min = min;Last, but not least, we can rename xxNEWtargetTemperature to targetTemperature:
diff --git HeatingPlan
- xxNEWtargetTemperature(selectedTemperature) {
+ targetTemperature(selectedTemperature) {
diff --git TemperatureManager
export function handleThermostatReading(thePlan, thermostat) {
- if (thePlan.xxNEWtargetTemperature(thermostat.selectedTemperature) > thermostat.currentTemperature) setToHeat();
- else if (thePlan.xxNEWtargetTemperature(thermostat.selectedTemperature) < thermostat.currentTemperature) setToCool();
+ if (thePlan.targetTemperature(thermostat.selectedTemperature) > thermostat.currentTemperature) setToHeat();
+ else if (thePlan.targetTemperature(thermostat.selectedTemperature) < thermostat.currentTemperature) setToCool();
else setOff();
}And that finishes off our work.
Below there's the commit history for the steps detailed above.
| Commit SHA | Message |
|---|---|
| 4d39669 | extract selectedTemperature variable at HeatingPlan.targetTemperature |
| 02b5f18 | introduce xxNEWtargetTemperature at HeatingPlan |
| cf7ca0e | use xxNEWtargetTemperature at HeatingPlan.targetTemperature |
| df7fa43 | inline selectedTemperature at HeatingPlan.targetTemperature |
| 7720516 | inline targetTemperature function at handleThermostatReading |
| 5c253c3 | remove unused targetTemperature getter at HeatingPlan |
| 900bba3 | remove thermostat import at HeatingPlan |
| 11fc72f | rename xxNEWtargetTemperature to targetTemperature at HeatingPlan |
For the full commit history for this project, check the Commit History tab.