paint-brush
How to Find the Stinky Parts of Your Code [Part XXVIII]by@mcsee
331 reads
331 reads

How to Find the Stinky Parts of Your Code [Part XXVIII]

by Maximiliano ContieriDecember 18th, 2022
Read on Terminal Reader
Read this story w/o Javascript
tldt arrow

Too Long; Didn't Read

Code smells are just hints of something that might be wrong. Most of these smells are not required to be fixed per se… (You should look into it, though.)

People Mentioned

Mention Thumbnail
featured image - How to Find the Stinky Parts of Your Code [Part XXVIII]
Maximiliano Contieri HackerNoon profile picture

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

    1. 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.

    Tags

    • Over Design

    Relations

    Code Smell 114 - Empty Class

    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

  1. 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).

Tags

  • 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 58 - Yo-yo Problem

Code Smell 37 - Protected Attributes

Code Smell 125 - 'IS-A' Relationship

Code Smell 58 - Yo-yo Problem

More Info

Coupling: The one and only problem

Wikipedia


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

  1. Import and implement trivial solutions
  2. 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.

Tags

  • 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

  1. 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.

Tags

  • 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 01 - Anemic Models

Code Smell 40 - DTOs

Code Smell 90 - Implementative Callback Events

Code Smell 78 - Callback Hell

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

  1. 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.

Tags

  • 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