How to Find the Stinky Parts of Your Code [Part XXVIII]
source link: https://hackernoon.com/how-to-find-the-stinky-parts-of-your-code-part-xxviii
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.
Code smells Are a Classic.
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.)
Previous Code Smells
You can find all the previous code smells (Part i - XXVii) here.
Let's continue...
Code Smell 136 - Classes With just One Subclass
-
Being generic and foreseeing the future is good (again).
TL;DR: Don't over-generalize
Problems
- Speculative Design
- Complexity
- Over-Engineering
Solutions
- Remove the abstract class until you get more examples
Context
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.
Sample Code
Wrong
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
Right
class Boss(object): def __init__(self, name): self.name = name # Bosses are concrete and can change mood
Detection
- [x]Automatic
This is very easy for our linters since they can trace this error at compile time.
Exceptions
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.
- Over Design
Relations
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
Conclusion
We need to wait for abstractions and not be creative and speculative.
Credits
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
Code Smell 137 - Inheritance Tree Too Deep
Yet another bad code reuse symptom
TL;DR: Favor composition over inheritance
Problems
- Coupling
- Subclassification Reuse
- Bad cohesion
- Fragile base classes
- Method overriding
- Liskov Substitution
Solutions
- Break clases and compose them.
Context
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.
Sample Code
Wrong
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
Right
classdef Horse
methods
// Horse behavior
end
end
Detection
- [x]Automatic
Many linters report Depth of inheritance tree (DIT).
- Hierarchies
Conclusion
Look after your hierarchies and break them often.
Relations
Code Smell 11 - Subclassification for Code Reuse
Code Smell 43 - Concrete Classes Subclassified
Code Smell 37 - Protected Attributes
Code Smell 125 - 'IS-A' Relationship
More Info
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
Code Smell 138 - Packages Dependency
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
Problems
Solutions
- Import and implement trivial solutions
- Rely on external and mature dependencies
Context
Recently, There's a trend to rely on a hard to trace dependencies.
This introduces coupling into our designs and architectural solutions.
Sample Code
Wrong
$ 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;
};
Right
function isOdd(value) {
const n = Math.abs(value);
return (n % 2) === 1;
};
// Just solve it inline
Detection
- [x]Automatic
We can check our external dependencies and stick to the minimum.
We can also depend on a certain concrete version to avoid hijacking.
- Security
Conclusion
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.
Relations
Code Smell 94 - Too Many imports
More Info
Credits
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
Code Smell 139 - Business Code in the User Interface
Validations should be on the interface, or not?
TL;DR: Always create correct objects in your back-ends. UIs are accidental.
Problems
- Security problems
- Code Duplication
- Testability
- Extensibility to APIs, micro-services, etc.
- Anemic and mutable objects
- Bijection Violation
Solutions
- Move your validations to the back-end.
Context
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.
Sample Code
Wrong
<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>
Right
<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>
Detection
- [x]Semi-Automatic
We can detect some behavior patterns in our UI code
Exceptions
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.
- Mutability
Conclusion
Use TDD.
You will put all your business logic behavior on your domain objects.
Relations
Code Smell 97 - Error Messages Without Empathy
Code Smell 90 - Implementative Callback Events
More Info
Credits
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
Code Smell 140 - Short Circuit Evaluation
We learn short circuits in our first programming courses. We need to remember why.
TL;DR: Be lazy when evaluating boolean conditions
Problems
- Side effects
- Bijection Fault
- Performance issues
Solutions
- Use a short circuit instead of a complete evaluation
Context
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.
Sample Code
Wrong
<?
if (isOpen(file) & size(contents(file)) > 0)
// Full evaluation
// Will fail since we cannot retrieve contents
// from not open file
Right
<?
if (isOpen(file) && size(contents(file)) > 0)
// Short circuit evaluation
// If file is not open it will not get the contents
Detection
- [x]Automatic
We can warn our developers when they use full evaluation.
Exceptions
Don't use short-circuit as an IF alternative.
if the operands have side effects, this is another code smell.
- Boolean
Conclusion
Most programming languages support short-circuits.
Many of them have it as the only option.
We need to favor these kinds of expressions.
Relations
Code Smell 101 - Comparison Against Booleans
Code Smell 69 - Big Bang (JavaScript Ridiculous Castings)
More Info
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
The Mobile Debugging Writing Contest is sponsored by our good friends at Sentry.
Prizes worth $1,000 can be won EVERY MONTH!! Use this template to enter the debugging writing contest.
Recommend
About Joyk
Aggregate valuable and interesting links.
Joyk means Joy of geeK