It smells because there are likely many instances where it could be edited or improved.
Most of these smells are just hints of something that might be wrong. Therefore, they are not required to be fixed per se… (You should look into it, though.)
You can find all the previous code smells (Part i - XXVii) here.
Let's continue...
Being generic and foreseeing the future is good (again).
TL;DR: Don't over-generalize
In the past, programmers told us to design for change.
Nowadays, We keep following the scientific method.
Whenever we find a duplication we remove it.
Not before.
Not with interfaces, not with classes.
class Boss(object):
def __init__(self, name):
self.name = name
class GoodBoss(Boss):
def __init__(self, name):
super().__init__(name)
# This is actually a very classification example
# Bosses should be immutable but can change their mood
# with constructive feedback
class Boss(object):
def __init__(self, name):
self.name = name
# Bosses are concrete and can change mood
This is very easy for our linters since they can trace this error at compile time.
Some frameworks create an abstract class as a placeholder to build our models over them.
Subclassifying should be never our first option.
A more elegant solution would be to declare an interface since it is less coupled.
Code Smell 11 - Subclassification for Code Reuse
Code Smell 43 - Concrete Classes Subclassified
Code Smell 92 - Isolated Subclasses Names
Code Smell 135 - Interfaces With just One Realization
We need to wait for abstractions and not be creative and speculative.
Photo by Benjamin Davies on Unsplash
Writing a class without its contract would be similar to producing an engineering component (electrical circuit, VLSI (Very Large Scale Integration) chip, bridge, engine...) without a spec. No professional engineer would even consider the idea.
Bertrand Meyer
Software Engineering Great Quotes
Yet another bad code reuse symptom
TL;DR: Favor composition over inheritance
Old papers recommended using classes as a specialization for code reuse.
We learnt that composition is a more efficient and extensible way to share behavior.
classdef Animalia
end
classdef Chordata < Animalia
end
classdef Mammalia < Chordata
end
classdef Perissodactyla < Mammalia
end
classdef Equidae < Perissodactyla
end
classdef Equus < Equidae
//Equus behaviour
end
classdef EFerus < Equus
//EFerus behaviour
end
classdef EFCaballus < EFerus
//EFCaballus behaviour
end
classdef Horse
methods
// Horse behavior
end
end
Many linters report Depth of inheritance tree (DIT).
Look after your hierarchies and break them often.
Code Smell 11 - Subclassification for Code Reuse
Code Smell 43 - Concrete Classes Subclassified
Code Smell 37 - Protected Attributes
Code Smell 125 - 'IS-A' Relationship
Coupling: The one and only problem
Software entities (classes, modules, functions, etc.) should be open for extension, but closed for modification.
Bertrand Meyer
Software Engineering Great Quotes
There's an industry trend to avoid writing code as much as possible. But this is not for free
TL;DR: Write your code unless you need an existing complex solution
Recently, There's a trend to rely on a hard to trace dependencies.
This introduces coupling into our designs and architectural solutions.
$ npm install --save is-odd
// https://www.npmjs.com/package/is-odd
// This package has about 500k weekly downloads
// https://github.com/i-voted-for-trump/is-odd/blob/master/index.js
module.exports = function isOdd(value) {
const n = Math.abs(value);
return (n % 2) === 1;
};
function isOdd(value) {
const n = Math.abs(value);
return (n % 2) === 1;
};
// Just solve it inline
We can check our external dependencies and stick to the minimum.
We can also depend on a certain concrete version to avoid hijacking.
Lazy programmers push reuse to absurd limits.
We need a good balance between code duplication and crazy reuse.
As always, there are rules of thumb but no rigid rules.
Code Smell 94 - Too Many imports
Photo by olieman.eth on Unsplash
Thanks to Ramiro Rela for this smell
Complexity kills. It sucks the life out of developers, it makes products difficult to plan, build and test, it introduces security challenges, and it causes end-user and administrator frustration.
Ray Ozzie
Software Engineering Great Quotes
Validations should be on the interface, or not?
TL;DR: Always create correct objects in your back-ends. UIs are accidental.
Code Duplication is a warning for premature optimization.
Building a system with UI validations might evolve to an API or external component consumption.
We need to validate objects on the back-end and send good validation errors to client components.
<script type="text/javascript">
function checkForm(form)
{
if(form.username.value == "") {
alert("Error: Username cannot be blank!");
form.username.focus();
return false;
}
re = /^\w+$/;
if(!re.test(form.username.value)) {
alert("Error: Username must contain only letters, numbers and underscores!");
form.username.focus();
return false;
}
if(form.pwd1.value != "" && form.pwd1.value == form.pwd2.value) {
if(form.pwd1.value.length < 8) {
alert("Error: Password must contain at least eight characters!");
form.pwd1.focus();
return false;
}
if(form.pwd1.value == form.username.value) {
alert("Error: Password must be different from Username!");
form.pwd1.focus();
return false;
}
re = /[0-9]/;
if(!re.test(form.pwd1.value)) {
alert("Error: password must contain at least one number (0-9)!");
form.pwd1.focus();
return false;
}
re = /[a-z]/;
if(!re.test(form.pwd1.value)) {
alert("Error: password must contain at least one lowercase letter (a-z)!");
form.pwd1.focus();
return false;
}
re = /[A-Z]/;
if(!re.test(form.pwd1.value)) {
alert("Error: password must contain at least one uppercase letter (A-Z)!");
form.pwd1.focus();
return false;
}
} else {
alert("Error: Please check that you've entered and confirmed your password!");
form.pwd1.focus();
return false;
}
alert("You entered a valid password: " + form.pwd1.value);
return true;
}
</script>
<form ... onsubmit="return checkForm(this);">
<p>Username: <input type="text" name="username"></p>
<p>Password: <input type="password" name="pwd1"></p>
<p>Confirm Password: <input type="password" name="pwd2"></p>
<p><input type="submit"></p>
</form>
<script type="text/javascript">
// send a post to a backend
// backend has domain rules
// backend has test coverage and richmodels
// it is more difficult to inject code in a backend
// Validations will evolve on our backend
// Business rules and validations are shared with every consumer
// UI / REST / Tests / Microservices ... etc. etc.
// No duplicated code
function checkForm(form)
{
const url = "https://<hostname/login";
const data = {
};
const other_params = {
headers : { "content-type" : "application/json; charset=UTF-8" },
body : data,
method : "POST",
mode : "cors"
};
fetch(url, other_params)
.then(function(response) {
if (response.ok) {
return response.json();
} else {
throw new Error("Could not reach the API: " + response.statusText);
}
}).then(function(data) {
document.getElementById("message").innerHTML = data.encoded;
}).catch(function(error) {
document.getElementById("message").innerHTML = error.message;
});
return true;
}
</script>
We can detect some behavior patterns in our UI code
If you have strong evidence on severe performance bottlenecks you need to automatically duplicate your business logic on the frontend.
You cannot just skip the backend part.
You should not make it manually because you will forget to do it.
Use TDD.
You will put all your business logic behavior on your domain objects.
Code Smell 97 - Error Messages Without Empathy
Code Smell 90 - Implementative Callback Events
Photo by Lenin Estrada on Unsplash
I think another good principle is separating presentation or user interface (UI) from the real essence of what your app is about. By following that principle I have gotten lucky with changes time and time again. So I think that's a good principle to follow.
Martin Fowler
Software Engineering Great Quotes
We learn short circuits in our first programming courses. We need to remember why.
TL;DR: Be lazy when evaluating boolean conditions
We learn booleans in our 101 computer courses.
Boolean's truth tables are great for mathematics, but we need to be more intelligent as software engineerings.
Short circuit evaluation helps us to be lazy and even build invalid full evaluations.
<?
if (isOpen(file) & size(contents(file)) > 0)
// Full evaluation
// Will fail since we cannot retrieve contents
// from not open file
<?
if (isOpen(file) && size(contents(file)) > 0)
// Short circuit evaluation
// If file is not open it will not get the contents
We can warn our developers when they use full evaluation.
Don't use short-circuit as an IF alternative.
if the operands have side effects, this is another code smell.
Most programming languages support short-circuits.
Many of them have it as the only option.
We need to favor these kinds of expressions.
Code Smell 101 - Comparison Against Booleans
Code Smell 69 - Big Bang (JavaScript Ridiculous Castings)
Writing a class without its contract would be similar to producing an engineering component (electrical circuit, VLSI (Very Large Scale Integration) chip, bridge, engine...) without a spec. No professional engineer would even consider the idea.
Bertrand Meyer
Software Engineering Great Quotes
Next article, 5 code smells more