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 - XXXVI) here.

Let's continue...

---

Code Smell 181 - Nested Classes

Nested or Pseudo-Private Classes seem great for hiding implementation details.

TL;DR: Don't use nested classes

Problems

• Bijection fault to real-world concepts.
• Lack of testability
• Lack of reuse
• Scopes and namespaces complexity

Solutions

1. Make the class public
2. Keep the public class under your own namespace/module.
3. Use a Facade to the external world to hide it.

Context

Some languages allow us to create private concepts that only live inside a more significant idea.

These classes are harder to test, harder to debug, and reuse.

Sample Code

Wrong

class Address {

  String description = "Address: ";

  public class City {
    String name = "Doha";
  }
}


public class Main {

  public static void main(String[] args) {

    Address homeAddress = new Address();
    Address.City homeCity = homeAddress.new City();
    System.out.println(homeAddress.description + homeCity.name);
  }
}

// The output is "Address: Doha"
//
// If we change privacy to 'private class City' 
//
// We get an error " Address.City has private access in Address"

Right

class Address {
  String description = "Address: ";
}

 
class City {
  String name = "Doha";
}


public class Main {
  public static void main(String[] args) {
    Address homeAddress = new Address();
    City homeCity = new City();
    System.out.println(homeAddress.description + homeCity.name);
  }
}

// The output is "Address: Doha"
//
// Now we can reuse and test the City concept

Detection

• [x]Automatic

Since this is a language feature, we can detect it and avoid its usage.

Tags

• Hierarchies

Conclusion

Many features are bloated with complex features.

We seldom need these new pop culture features.

We need to keep a minimal set of concepts.

More Info

• W3 schools

Disclaimer

Code Smells are just my opinion.

Credits

Photo by Dana Ward on Unsplash

---

Developers are drawn to complexity like moths to a flame, frequently with the same result.

Neal Ford

Software Engineering Great Quotes

---

Code Smell 182 - Over Generalization

We are refactoring fans. Sometimes we need to stop and think.

TL;DR: Don't make generalizations beyond real knowledge.

Problems

• Overgeneralization
• Bijection violation

Solutions

1. Think before making structural generalizations

Context

Refactoring is not just looking at structural code.

We need to refactor behavior and check if it needs an abstraction.

Sample Code

Wrong

fn validate_size(value: i32) {
    validate_integer(value);
}

fn validate_years(value: i32) {
    validate_integer(value);
}

fn validate_integer(value: i32) {
    validate_type(value, :integer);
    validate_min_integer(value, 0);
}

Right

fn validate_size(value: i32) {
 	validate_type(value, Type::Integer);
	validate_min_integer(value, 0);
}
	
fn validate_years(value: i32) {
	validate_type(value, Type::Integer);
	validate_min_integer(value, 0);
}
	
// Duplication is accidental, therefore we should not abstract it	

Detection

• [x]Manual

This is a semantic smell.

Tags

• Duplication

Conclusion

Software development is a thinking activity.

We have automated tools to help and assist us. We need to be in charge.

Relations

Code Smell 46 - Repeated Code

More Info

• DRY—The Evils of Duplication

Disclaimer

Code Smells are just my opinion.

Credits

Photo by Matthew Henry on Unsplash

---

Duplication is far cheaper than the wrong abstraction.

Sandi Metz

---

Code Smell 183 - Obsolete Comments

Comments are a code smell. Obsolete comments are a real danger and nobody maintains what can't be executed.

TL;DR: Don't trust comments. They are dead.

Problems

• Bad documentation
• Maintainability

Solutions

1. Replace comments with tests

Refactorings

Refactoring 005 - Replace Comment with Function Name

Context

We know comments add almost no value to our code.

We need to restrict comments only to very important decisions.

Since most people change logic and forget to update comments they might become obsolete easily.

Sample Code

Wrong

void Widget::displayPlugin(Unit* unit)
{

 	// TODO the Plugin will be modified soon, 
	// so I don't implement this right now

 	if (!isVisible) {
		// hide all widgets
		return;
 	}

}

Right

void Widget::displayPlugin(Unit* unit)
{ 
	if (!isVisible) {
		return;
 	}
}

Detection

• [x]Semi-Automatic

We can warn for comments in our code and try to remove them.

Exceptions

• Very important design decisions

Tags

• Comments

Conclusion

We need to think before adding a comment. Once It is in the codebase is beyond our control and can start to lie anytime.

Relations

Code Smell 05 - Comment Abusers

Code Smell 152 - Logical Comment

Code Smell 151 - Commented Code

Disclaimer

Code Smells are just my opinion.

Credits

Photo by Volodymyr Hryshchenko on Unsplash

---

Obsolete comments tend to migrate away from the code they once described. They become floating islands of irrelevance and misdirection in the code.

Bob Martin

Software Engineering Great Quotes

---

Code Smell 184 - Exception Arrow Code

Arrow code is a code smell. Exception polluting is another. This is a mortal combination.

TL;DR: Don't cascade your exceptions

Problems

• Readability
• Complexity

Solutions

1. Rewrite the nested clauses

Context

In the same way arrow code is hard to read, handling exceptions is a usual case when we must address the topics in a cascade way.

Sample Code

Wrong

class QuotesSaver {
    public void Save(string filename) {
        if (FileSystem.IsPathValid(filename)) {
            if (FileSystem.ParentDirectoryExists(filename)) {
                if (!FileSystem.Exists(filename)) {
                    this.SaveOnValidFilename(filename);
                } else {
                    throw new I0Exception("File exists: " + filename);
                }
            } else {
                throw new I0Exception("Parent directory missing at " + filename);
            }
        } else {
            throw new ArgumentException("Invalid path " + filename);
        }
    }
}

Right

public class QuotesSaver {
    public void Save(string filename) {
        if (!FileSystem.IsPathValid(filename)) {
            throw new ArgumentException("Invalid path " + filename);
        } else if (!FileSystem.ParentDirectoryExists(filename)) {
            throw new I0Exception("Parent directory missing at " + filename);
        } else if (FileSystem.Exists(filename)) {
             throw new I0Exception("File exists: " + filename);
        }
        this.SaveOnValidFilename(filename);
    }
}

Detection

• [x]Semi-Automatic

Some linters warn us when we have this kind of complexity

Tags

• Exceptions

Conclusion

Exceptions are less critical than normal cases.

If we need to read more exceptional code than normal then it is time to improve our code.

Relations

Code Smell 102 - Arrow Code

Code Smell 26 - Exceptions Polluting

Disclaimer

Code Smells are just my opinion.

Credits

Photo by Remy Gieling on Unsplash

---

An error doesn't become a mistake until you refuse to correct it.

Orlando Aloysius Battista

Software Engineering Great Quotes

---

Code Smell 185 - Evil Regular Expressions

Regular expressions are a code smell. Sometimes also a vulnerability

TL;DR: Try to minimize Regular Expression's recursive rules.

Problems

• Security Issues
• Readability
• Premature Optimization

Solutions

1. Cover the cases with tests to see if they halt
2. Use algorithms instead of regular expressions
3. Add timeout handlers

Context

This is known as ReDos attack, a subtype of a Denial of Service attack.

ReDoS attacks can be divided into two types:

A string with an evil pattern is passed to an application. Then this string is used as a regex, which leads to ReDoS.

A string with a vector attack format is passed to an application. Then this string is evaluated by a vulnerable regex, which leads to ReDoS.

Sample Code

Wrong

package main

import (
    "regexp"
    "fmt"
)

func main() {
    var re = regexp.MustCompile(`^(([a-z])+.)+[A-Z]([a-z])+$`)
    var str = `aaaaaaaaaaaaaaaaaaaaaaaa!`
    
    for i, match := range re.FindAllString(str, -1) {
        fmt.Println(match, "found at index", i)
    }
}

---

Right

package main

import (
    "fmt"
    "strings"
)

func main() {
    var str = `aaaaaaaaaaaaaaaaaaaaaaaa!`
    
    words := strings.Fields(str)
    
    for i, word := range words {
        if len(word) >= 2 && word[0] >= 'a' && word[0] <= 'z' && word[len(word)-1] >= 'A' 
            && word[len(word)-1] <= 'Z' {
                fmt.Println(word, "found at index", i)
        }
    }
}

Detection

• [x]Semi-Automatic

Many languages avoid this kind of regular expression.

We can also scan the code for this vulnerability.

Tags

• Security

Conclusion

Regular Expressions are tricky and hard to debug.

We should avoid them as much as possible.

Relations

Code Smell 41 - Regular Expression Abusers

More Info

Catastrophic backtracking: how can a regular expression cause a ReDoS vulnerability?

https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS

Runaway Regular Expressions: Catastrophic Backtracking

Disclaimer

Code Smells are just my opinion.

Credits

Photo by engin akyurt on Unsplash

---

Some people, when confronted with a problem, think “I know, I’ll use regular expressions.” Now they have two problems.

Jamie Zawinski

---

5 more code smells are coming soon…