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

Your code 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 - XLIV) here.

Let's continue...

Code Smell 221 - Missing Break in Switch

You abuse cases in switches and make subtle mistakes

TL;DR: Cases are GOTOs, but you might be missing them

Problems

• Hidden defects
• Readability

Solutions

1. Add the missing break
2. Convert the switch into a polymorphic hierarchy
3. Remove the default switch

Context

In a switch statement, when a match is found in a particular case, the code execution will start from that case and continue executing all subsequent cases until a break statement is encountered or the switch block ends.

This behavior is called "fall-through."

Forgetting a break clause will continue with the following case.

This is similar to a very serious vulnerability that implied an urgent release.

Sample Code

Wrong

  switch (number) {
      case 1:
          printf("Number is 1.\n");
          break;
      case 2:
          printf("Number is 2.\n"); 
          // Missing break
      case 3:
          // Case 2 will continue here
          printf("Number is 3.\n"); 
          break;
      default:
          printf("Number is not 1, 2, or 3.\n");
  }

// If the number is 2 this will output numbers 2 and 3

Right

  switch (number) {
      case 1:
          printf("Number is 1.\n");
          break;
      case 2:
          printf("Number is 2.\n"); 
          break; // Added 'break' to prevent fall-through
      case 3:
          printf("Number is 3.\n"); 
          break;
      default:
          printf("Number is not 1, 2, or 3.\n");
  }

// This is correct even though switches AND defaults
// Are other code smells

Detection

• [x]Automatic

Many linters and also ChatGPT detect this smell.

Tags

• IFs

Conclusion

Using switches and causes is problematic, your need to use higher-level sentences.

Relations

Code Smell 110 - Switches With Defaults

Code Smell 36 - Switch/case/elseif/else/if statements

Code Smell 100 - GoTo

More Info

Sonar Source

Common Weakness Enumeration

How to Get Rid of Annoying IFs Forever

Apple's Security Defect

I am not terribly dogmatical about the goto statement. I have the uncomfortable feeling that others are making a religion out of it, as if the conceptual problems of programming could be solved by a single trick, by a simple form of coding discipline!

- Edsger Dijkstra

Code Smell 222 - Comma Operator

Don't abuse this fancy operator

TL;DR: Use comma operator just for loops

Problems

• Readability
• Hidden Defects

Solutions

1. Avoid operator usage
2. Prefer foreach operator
3. Break the sentences

Context

In JavaScript, the comma operator allows you to evaluate multiple expressions sequentially and return the value of the last expression.

It's denoted by a comma and separates multiple expressions within a larger expression.

Each expression is evaluated in order from left to right, and the final value of the entire comma-separated expression is the value of the last expression.

Sample Code

Wrong

const gravitationalConstant = 6.67430e-11;

const massBlackHole1 = 1.5e31;  // Mass of the first black hole in kg
const massBlackHole2 = 2.2e32;  // Mass of the second black hole in kg
const distanceBlackHoles = 5.7e20;  // Distance between black holes in meters

var force = (distanceSquared = distanceBlackHoles * distanceBlackHoles,
             (gravitationalConstant * massBlackHole1 * massBlackHole2) /
             distanceSquared);
// Two operations in a single statement with comma operator  

Right

function calculateGravitationalForce(mass1, mass2, distance) {
  const gravitationalConstant = 6.67430e-11;
  return (gravitationalConstant * mass1 * mass2) / (distance * distance);  
}

const massBlackHole1 = 1.5e31;  // Mass of the first black hole in kg
const massBlackHole2 = 2.2e32;  // Mass of the second black hole in kg
const distanceBlackHoles = 5.7e20;  // Distance between black holes in meters

const force = calculateGravitationalForce(
  massBlackHole1,
  massBlackHole2,
  distanceBlackHoles
);
// Notice force is calculated with a separate function

Detection

• [x]Automatic

Many linters can detect this problem.

Exceptions

• For loops are a valid exception dough they might lead you to another code smell

Tags

• Readability

Conclusion

This valid operator was designed to shorten for loops but is now sometimes abused.

My computer's so fast it finishes an infinite loop in 5 minutes.

- Chisel Wright

Code Smell 223 - Racial Naming

Software evolves, and so does culture.

TL;DR: Avoid old terms like whitelists, blacklists, master, etc.

Problems

• Racial Connotations
• Exclusionary Language
• Diverse Perspectives

Solutions

1. Use alternative terminology

Context

Language evolves, and technical terms should follow it.

You can change racial names with alternative terminology:

• Allowlist: Replace "whitelist" with "allowlist." This term maintains the intended meaning without racial connotations.
• Denylist: Substitute "blacklist" with "denylist." This term conveys the same concept without perpetuating negative racial associations.
• Permit List and Block List: Another option is to use "permit list" in place of "whitelist" and "block list" instead of "blacklist."
• Main branches: You can replace "master" with "main".
• Master/Slave: You can replace them with Primary/Replica/Mirror etc.

Sample Code

Wrong

val whitelist = listOf("Barbie", "Ken")

val blacklist = listOf("Midge")

val gitCommand = "git pull origin master"

val process = Runtime.getRuntime().exec(gitCommand)

Right

val allowlist = listOf("Barbie", "Ken")

val denylist = listOf("Midge")

val gitCommand = "git pull origin main"

val process = Runtime.getRuntime().exec(gitCommand)

Detection

• [x]Semi-Automatic

You can set up a denylist (not a blacklist) of terms you need to double-check for accuracy.

Exceptions

• References to old manuals

Tags

• Naming

Conclusion

Just as we refactor code to enhance its quality, we should also refactor our language and terminology to promote inclusivity and diversity.

By eliminating racially insensitive terms like "whitelist" and "blacklist" in favor of more inclusive alternatives, we contribute to a more equitable and welcoming tech industry. Let's embrace change and create a coding environment where everyone feels valued, regardless of their background or ethnicity.

Relations

Code Smell 105 - Comedian Methods

More Info

Prevalence of racist language in discussions of predatory publishing

Wikipedia

Rename master to main

An ultimate joint challenge for the biological and the computational sciences is the understanding of the mechanisms of the human brain, and its relationship with the human mind.

- Tony Hoare

Code Smell 224 - Deodorant Comments

You use nice words to excuse bad code

TL;DR: Don't excuse bad code. Write a clean one!

Problems

• Readability

Solutions

1. Rewrite the code and delete the comment

Context

The term comes from Martin Fowler's book "Refactoring: Improving the Design of Existing Code"

Sample Code

Wrong

# This is a function that adds two numbers
def s(a, b):
    # Now you are going to add a and b
    res = a + b
    # And return the result
    return res

Right

def sum(adder, anotherAdder):
    
    return adder + anotherAdder

If you ask ChatGPT to improve this version it will actually worsen it:

def calculate_sum(number1, number2):
    # Calculate the sum of two numbers
    result = number1 + number2
    return result

# In this improved version:
#
# The function name calculate_sum is more descriptive than sum, 
# making it clear that this function calculates the sum of two numbers.
# (Wrong) it is more imperative and mistakes the 'what' with the 'how'
#
# The parameter names number1 and number2 are more meaningful 
# than adder and anotherAdder, helping to indicate the purpose of each parameter.
# (wrong) They indicate type instead of role
#
# The comment # Calculate the sum of two numbers provides a clear 
# and concise explanation of what the function does, 
# making it easier for someone reading the code to understand its purpose.    
# (wrong) in fact, it is an example of deodorant and useless comment

Detection

• [x]Semi-Automatic

Most comments are code smells.

You can remove deodorant comments and improve the code.

Exceptions

• Comments should only be used to describe important design decisions.

Tags

• Comments

Conclusion

Remove any meaningless comment you find in your code.

Relations

Code Smell 151 - Commented Code

Code Smell 183 - Obsolete Comments

Code Smell 146 - Getter Comments

Code Smell 05 - Comment Abusers

Refactoring 011 - Replace Comments with Tests

More Info

Clean Code In C#

The reason we mention comments here is that comments often are used as a deodorant. It's surprising how often you look at thickly commented code and notice that the comments are there because the code is bad.

- Martin Fowler

Code Smell 225 - Pass by Reference

Pass by copy, pass by reference. which is better?

TL;DR: Beware of passing arguments by reference

Problems

• Unexpected Results
• Side Effects
• Readability
• Broken Encapsulation

Solutions

1. Pass arguments by copying even large objects. Don't make premature optimizations.
2. Declare variables as constants
3. Refactor the code
4. Make objects immutable to avoid accidental changes

5 Use Pure Functions

Context

A call-by-reference language like C# or PHP makes it more difficult for a programmer to track the effects of a function call and may introduce subtle bugs. This is a very old technique present in low-level languages to favor performance and avoid the cost of copying large structures.

Some languages like Go use pass-by-value semantics. When you pass arguments to a function, copies are made. However, when you pass a pointer to an object, you can modify the original object within the function. This is another code smell. On the contrary, functional languages forbid this mechanism completely.

Sample Code

Wrong

using System;
 
namespace Example
{
     class Betelgeuse
     {
         static void Main(string[] args)
         {
             double starSize = 100.0;
             Console.WriteLine("star size: {0}", starSize);
             // star size: 100
             double supernovaSize = SimulateFinalSize(ref starSize);
             // Notice 'ref' modifier
             Console.WriteLine("supernova size: {0}", supernovaSize); 
             // supernova size: 10000
             Console.WriteLine("original star size after: {0}", starSize);
             // original star size after: 10000
             // WRONG: It should not be affected
         }
         public static double SimulateFinalSize(ref double size)
         {
             // Notice 'ref' modifier
             // Oversimplification
             // You should use Sedov-Taylor solution
              size = size * 100;
              return size;
         }
     }
}

Right

using System;
 
namespace Example
{
     class Betelgeuse
     {
         static void Main(string[] args)
         {
             const double starSize = 100.0; 
             // The const modifier warns the compiler
             Console.WriteLine("star size: {0}", starSize);
             // star size: 100
             double supernovaSize = SimulateFinalSize(starSize);
             // Notice 'ref' is omitted
             Console.WriteLine("supernova size: {0}", supernovaSize);
             // supernova size: 10000
             Console.WriteLine("original star size after: {0}", starSize);
             // original star size after: 100
             // It remains at the original value
         }
         public static double SimulateFinalSize(double size)
         {
             // Notice 'ref' is omitted
             // Oversimplification
             // You should use Sedov-Taylor solution
              size = size * 100;
              return size;
         }
     }
}

Detection

• [x]Semi-Automatic

You can use many linters to warn with arguments passed by reference

Tags

• Readability

Conclusion

Passing objects by reference can lead to unexpected side effects if the function modifies the object in a way that wasn't anticipated by the caller.

You should use copy by value instead.

Relations

Code Smell 116 - Variables Declared With 'var'

Code Smell 176 - Changes in Essence

Code Smell 209 - Side Effects

More Info

Modifiyng Method Parameter

Wikipedia

Make it correct, make it clear, make it concise, make it fast. In that order.

Wes Dyer

Next week, 5 more smells