Code Smell 158 - Variables not Variable
You assign a value to a variable and use it, but never change it
TL;DR: Be declarative on mutability.
Problems
Readability
Honor the Bijection mutability.
Potential performance and memory issues.
Solutions
- Change the variable to a constant and be clear on its scope
Context
We are always learning from the domain.
Sometimes we guess that a value can change with the MAPPER.
Later on, we learn it won't change.
Therefore we need to promote it to a constant.
This will also avoid Magic Constants
Sample Code
Wrong
<?php
function configureUser() {
$password = '123456';
// Setting a password on a variable is another vulnerability
// And Code Smell
$user = new User($password);
}
Right
<?php
define("USER_PASSWORD", '123456')
function configureUser() {
$user = new User(USER_PASSWORD);
}
// or
function configureUser() {
$user = new User(userPassword());
}
function userPassword() : string {
return '123456';
}
// Case is an oversimplification as usual
Detection
[X] Automatic
Many linters check if the variable has just one assignment.
We can also perform mutation testing and try to modify the variable to see if tests break.
Tags
- Mutability
Conclusion
We must challenge ourselves and refactor when the variable scope is clear and we learn more about its properties and mutability.
Relations
Refactorings
More Info
Disclaimer
Code Smells are just my opinion.
Credits
Photo by Noah Buscher on Unsplash
A complex system that works is invariably found to have evolved from a simple system that worked.
John Gall
This article is part of the CodeSmell Series.
About the author
I’m a senior software engineer loving clean code, and declarative designs. S.O.L.I.D. and agile methodologies fan.
Comments
Join the discussionAs my own study of clean code progresses, I seem to occasionally find conflicting theories. Lately, I'm forming a solid preference to code to human-readability and to that end, I'm now almost universally giving 'names' to my control structures:
if ($SatInDriverSeat && $SeatbeltIsOn && $CarIsStarted) {
DriveCar();
}
becomes
$ReadyToDrive = ($SatInDriverSeat && $SeatbeltIsOn && $CarIsStarted);
if ($ReadyToDrive) {
DriveCar();
}
Clearly, $ReadyToDrive is a single-use variable, but the benefit it offers to code simplicity and readability outweighs the tiny potential performance losses of assigning a single-use variable, especially when used in a small, efficient method of a few lines, where the limited scope implies that its resources will be released as soon as the method has finished processing.
In fact, where possible, I'm now adopting the policy that any control or loop will only consist of a variable and a method call, for example,
foreach (['cat, 'dog', 'mouse'] as $animal) {
feed($animal);
}
becomes
$animals = ['cat', 'dog', 'mouse'];
foreach ($animals as $animal) { feed($animal); } ```
Code Smells
In this series, we will see several symptoms and situations that make us doubt the quality of our developments. We will present possible solutions. Most are just clues. They are no hard rules.
