Yet more code smells? Aren't them enough?
We see several symptoms and situations that make us doubt the quality of our development.
Let's look at some possible solutions.
Most of these smells are just hints of something that might be wrong. They are not rigid rules.
This is part V. Part I can be found here, Part II here, Part III is here, Part IV here, part V, VI, VII, VIII, IX and the last one (for now).
Let's continue...
Code Smell 51 - Double Negatives
Not operator is our friend. Not not operator is not our friend.
Problems
- Readability
Solutions
- Name your variables, methods and classes with positive names.
Sample Code
Wrong
if ( !work.isNotFinished() )Right
if ( work.isDone() )Detection
This is a semantic smell. We need to detect it on code reviews.
We can tell linters to check for Regular Expressions like !not or !isNot etc as a warning.
Tags
- Readability
Conclusion
Double negation is a very basic rule we learn as junior developers.
There are lots of production systems filled with this smell.
We need to trust our test coverage and make safe renames and other refactors.
More info
- What Exactly Is A Name: Rehab [Part II]
- Remove Double Negative
- Knot-of-nots: Avoiding negative names for boolean methods
Credits
Photo by Daniel Herron on Unsplash
It’s harder to read code than to write it.
Joel Spolsky
Code Smell 52 - Fragile Tests
Tests are our safety nets. If we don’t trust on their integrity, we will be in great danger.
Problems
- Determinism
- Confidence loss
- Wasted time
Solutions
- Test should be in full control. There should be no space for erratic behavior and degrees of freedom.
- Remove all tests coupling.
Coupling: The One and Only Software Designing Problem
Examples
Fragile, Intermittent, Sporadic or Erratic tests are common in many organizations.
Nevertheless, they mine the developers trust.
We must avoid them.
Sample Code
Wrong
import static org.junit.Assert.assertEquals;
import org.junit.Test;
import components.set.Set;
import components.set.Set1L;
public abstract class SetTest {
protected abstract Set<String> constructor();
@Test
public final void testAddEmpty() {
Set<String> s = this.constructor();
s.add("green");
s.add("blue");
assertEquals("{green. blue}", s.toString());
//This is fragile since it dependes on set sort (which is not defined)
}
}
Right
import static org.junit.Assert.assertEquals;
import org.junit.Test;
import components.set.Set;
import components.set.Set1L;
public abstract class SetTest {
protected abstract Set<String> constructor();
@Test
public final void testAddEmpty() {
Set<String> s = this.constructor();
s.add("green");
assertEquals("{green}", s.toString());
}
@Test
public final void testEntryAtSingleEntry() {
Set<String> s = this.createFromArgs("red");
Boolean x = s.contains("red");
assertEquals(true, x);
}
}
Detection
Detection can be done with test run statistics.
It is very hard to put some test in maintenance since we are removing a safety net.
More Info
Tags
- Coupling
- Determinism
Conclusion
Fragile tests show system coupling and not deterministic or erratic behavior.
Developers spend lots of time and effort fighting against this false positives.
The amateur software engineer is always in search of magic.
Grady Booch
Code Smell 53 — Explicit Iteration
We learned loops back in school. But enumerators and iterators are the next generation.
Photo by Elena Mozhvilo on Unsplash
Problems
- Encapsulation.
- Declarativeness.
Solutions
- Favor foreach() or high order iterators
- You will be able to use yield(), caches, proxies, lazy loading and much more when you hide your implementation details.
Sample Code
Wrong
for (i = 0; i < colors.count(), i++) {
print(colors[i]);
}
Right
foreach (color of colors) {
print(color);
}
//Closures and arrow functions
colors.foreach(color => print(color));
Detection
Linters can find this smell using regex.
There might be false positives. See exceptions below.
Exceptions
If the problem domain needs the elements to be bijected to natural numbers like indices, the first solution is adequate.
Remember all time to find real world analogies.
The One and Only Software Design Principle
Tags
- Declarative
Conclusion
This kind of smell do not ring the bell to many developers because they think this is a subtlety.
Clean code is full of this few declarative things that can make a difference.
More info
If you get tired of writing for loops, take a break and continue later.
David Walker
Code Smell 54 — Anchor Boats
Code is there. Just in case. We might need it soon.
Photo by Kris Mikael Krister on Unsplash
Problems
- Complexity
- Coupling
Solutions
- Remove dead code.
- Leave covered and real tested code.
Sample Code
Wrong
<?
final class DatabaseQueryOptimizer {
public function selectWithCriteria($tableName, $criteria) {
//Make some optimizations manipulating criterias
}
private function sqlParserOptimization(SQLSentence $sqlSentence): SQLSentence {
//Parse the SQL converting it to an string and then working with their nodes as strings and lots of regex
//This was a very costly operation overcoming real SQL benefits.
//But since we made too much work we decide to keep the code.
}
}
Right
<?
final class DatabaseQueryOptimizer {
public function selectWithCriteria($tableName, $criteria) {
//Make some optimizations manipulating criterias
}
}
Detection
Using some mutation testing variants we can remove the dead code and see it test fails.
We need to have good coverage to rely on this solution.
Tags
- YAGNI
Conclusion
Dead code is always a problem.
We can use modern development techniques like TDD to ensure all code is alive.
We all love T.D.D. We know its benefits, we have read a thousand tutorials on how to build a system using this…
More info
It is very hard to predict, especially the future.
Niels Bohr
Code Smell 55 — Object Orgy
If you see your objects as data holders you will violate their encapsulation, but you shouldn’t, as in real life, you should always ask for consent.
Picture by Nicolas Poussin
Problems
- Information Hiding Violation
- Encapsulation Violation
- Coupling
Solutions
- Couple to interfaces and behavior, never data.
Sample Code
Wrong
<?
final class Point {
public $x;
public $y;
}
final class DistanceCalculator {
function distanceBetween(Point $origin, Point $destination) {
return sqrt((($destination->x - $origin->x) ^ 2) + (($destination->y - $origin->y) ^ 2));
}
}
Right
<?
final class Point {
private $rho;
private $theta;
public function x() {
return $this->rho * cos($this->theta);
}
public function y() {
return $this->rho * sin($this->theta);
}
}
final class DistanceCalculator {
function distanceBetween(Point $origin, Point $destination) {
return sqrt((($destination->x() - $origin->x() ^ 2) + (($destination->y() - $origin->y()) ^ 2)));
}
}
Detection
You can set your linters to warn you for public attributes, setters and getters usage and discourage them.
Tags
- Coupling
Conclusion
If your classes are polluted with setters, getters and public methods you will certainly have ways to couple to their accidental implementation.
Also Known as
- Inappropriate intimacy
More info
A data structure is just a stupid programming language.
Bill Gosper
We are done for now. But we are pretty sure we will come across even more smells very soon!
I keep getting more suggestions on twitter, so they won't be the last!