277 reads

How to Find The Stinky Parts of Your Code [Part XI]

by Maximiliano ContieriJune 14th, 2021

Too Long; Didn't Read

Part I can be found here, Part II, Part I, Part III, Part IV, Part V, Part VI, Part VII, VII, VIII, IX and the last one (for now) This is part V. I'm senior software engineer specialized in declarative designs and S.O.L.D. and Agile lover. We see several symptoms and situations that make us doubt the quality of our development. We need to use linters to check for Regular Expressions like!not or!isNot as a warning.

People Mentioned

Company Mentioned

featured image - How to Find The Stinky Parts of Your Code [Part XI]

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.

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

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

How to Avoid Fragile Unit Tests?

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

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

What is (wrong with) software?

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.

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)));
    }

}