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. They are not required fixed per se… (You should look into it though.) Previous Code Smells Part I Part II Part III Part IV Part V Part VI Part VII Part VIII Part IX Part X Part XI Part XII Part XIII Part XIV Part XV Part XVI Part XVII Part XVIII Part XIX Part XX Part XXI Part XXII Let's continue... Code Smell 111 - Modifying Collections While Traversing Changing a collection while traversing might lead to unexpected errors TL;DR: Do not modify collections while traversing them Problems Unexpected Results Concurrency problems Solutions Avoid altering the collections Make collection copies Context We over-optimize our solutions with the prejudice that copying collections are expensive. This is not true for small and medium-size collections. Languages iterate collections in many different ways. Modifying them is generally not safe. Sample Code Wrong Collection<Integer> people = new ArrayList<>();
// here we add elements to the collection...
  
for (Object person : people) {
    if (condition(person)) {
        people.remove(person);
    }
}
// We iterate AND remove elements Right Collection<Integer> people = new ArrayList<>();
// Here we add elements to the collection...

List<Object> iterationPeople = ImmutableList.copyOf(people);
    
for (Object person : iterationPeople) {
    if (condition(person)) {
        people.remove(person);
    }
}
// We iterate a copy and remove it from original

coll.removeIf(currentIndex -> currentIndex == 5);
// Or use language tools (if available) Detection Semi Automatic [x] Many languages provide control both in compile and run-time Tags Fail Fast Conclusion This is something we learn in our first courses. It happens a lot in the industry and real-world software Relations Code Smell 53 - Explicit Iteration More Info Stack Overflow Credits Photo by on Christine Roy Unsplash Bugs are bugs. You write code with bugs because you do. If it’s a safe language in the sense of run-time safe, the operating system crashes instead of doing a buffer overflow in a way that’s exploitable. - Ken Thompson Software Engineering Great Quotes Code Smell 112 - Testing Private Methods If you work on unit testing, sooner or later you will face this dilemma TL;DR: Don't test your private methods. Problems Breaking Encapsulation Code Duplication Solutions If your method is simple, you don't need to test it. If your method is complicated, you need to convert it into a Method Object. Do not make your methods public for testing. Do not to avoid protection. use metaprogramming Do not move the private computation to . helpers Do not use for computations. static methods Context We test our classes and methods. At some point, we rely on auxiliary computations and we need to test them in a white-box way. Sample Code Wrong <?

final class Star {
  
  private $distanceInParsecs;
  
  public function timeToReachLightToUs() {
    return $this->convertDistanceInParsecsToLightYears($this->distanceInParsecs);
  }
  
  private function convertDistanceInParsecsToLightYears($distanceInParsecs) {
      return 3.26 * $distanceInParsecs;
      // function is using an argument which is already available.
      // since it has private access to $distanceInParsecs
      // this is another smell indicator.

      // We cannot test this function since it is private
  }
} Right <?

final class Star {
  
  private $distanceInParsecs;   
  
  public function timeToReachLightToUs() {
    return new ParsecsToLightYearsConverter($this->distanceInParsecs);
  }
}

final class ParsecsToLightYearsConverter {
  public function convert($distanceInParsecs) {
      return 3.26 * $distanceInParsecs;
  }
}

final class ParsecsToLightYearsConverterTest extends TestCase {
  public function testConvert0ParsecsReturns0LightYears() {
    $this->assertEquals(0, (new ParsecsToLightYearsConverter)->convert(0));
  }
    // we can add lots of tests and rely on this object
    // So we don't need to test Star conversions.
    // We can yet test Star public timeToReachLightToUs()
    // This is a simplified scenario

} Detection Semi-Automatic [x] This is a semantic smell. We can only find metaprogramming abuse on some unit frameworks. Tags Test Smells Conclusion With this guide, we should always choose the method object solution. Relations Code Smell 21 - Anonymous Functions Abusers Code Smell 22 - Helpers Code Smell 18 - Static Functions More Info Testing Private Methods Guide Laziness I - Metaprogramming Credits Photo by on Dan Nelson Unsplash Just as it is a good practice to make all fields private unless they need greater visibility, it is a good practice to make all fields final unless they need to be mutable. - Brian Goetz Software Engineering Great Quotes Code Smell 113 - Data Naming Use entity domain names to model entity domain objects. TL;DR: Don't name your variables as Data. Problems Readability Bad Naming Solutions Use role suggesting names. Find names in the . Bijection Context We use 'data' a lot in our variables. We are used to doing it. Using this kind of name favors the anemic treatment of objects. We should think about domain-specific and role-suggesting names. Sample Code Wrong if (!dataExists()) {
  return '<div>Loading Data...</div>';
} Right if (!peopleFound()) {
  return '<div>Loading People...</div>';
} Detection Semi-Automatic [x] We can check for this substring on our code and warn our developers. Tags Readability Naming Conclusion Data is everywhere if you see the world as only data. We can never see the data we manipulate. We can only infer it through behavior. We don't know the current temperature. We observe our thermometer pointing at 35 Degrees. Our variables should reflect the domain and role they are fulfilling. Naming them as 'data' is lazy and hinders readability. Relations Code Smell 01 - Anemic Models Code Smell 65 - Variables Named after Types More Info What exactly is a name - Part II Rehab Twenty percent of all input forms filled out by people contain bad data. - Dennis Ritchie Software Engineering Great Quotes Code Smell 114 - Empty Class Have you encountered classes without behavior? Classes are their behavior. TL;DR: Remove all empty classes. Problems Fault Bijection Namespace Polluting Classes used as DTOs Classes used as global references Solutions Remove the classes and replace them with objects instead. If your classes are Anemic Exceptions, . remove them Context Many developers still think classes are . data repositories They couple concepts with . different behavior returning different data Sample Code Wrong class ShopItem { 
  code() { }
  description() { }                 
}

class BookItem extends ShopItem { 
   code() { return 'book' }
   description() { return 'some book'}     
}

// concrete Class has no real behavior, just return different 'data' Right class ShopItem { 
  constructor(code, description) {
    // validate code and description
    this._code = code;
    this._description = description;
  }
  code() { return this._code }
  description() { return this._description }                 
  // dd more functions to avoid anemic classes
  // getters are also code smells, so we need to iterate it
}

bookItem = new ShopItem('book', 'some book);
// create more items Detection Automatic [x] Several linters warn us of empty classes. We can also make our own scripts using . metaprogramming Tags Behavior Conclusion Classes are what they do, their behavior. Empty classes do nothing. Relations Code Smell 26 - Exceptions Polluting Code Smell 40 - DTOs Code Smell 60 - Global Classes Code Smell 01 - Anemic Models More Info The one and only Design Principle Refactoring 004 - Remove Unhandled Exceptions Credits Photo by on Kelly Sikkema Unsplash An error arises from treating object variables (instance variables) as if they were data attributes and then creating your hierarchy based on shared attributes. Always create hierarchies based on shared behaviors, side. - David West Software Engineering Great Quotes Code Smell 115 - Return True Booleans are natural code smells. Returning and casting them is sometimes a mistake. TL;DR: Don't return true or false. Be declarative. Problems Readability Primitive Obsession If/Else abuse Solutions Return truth value in a declarative way Replace With polymorphism. IF Context Dealing with low-level abstractions, we usually return booleans. When we create complex and mature software, we start to forget about this primitive obsession and care about rules and identities. real-world Sample Code Wrong boolean isEven(int num) {
     if(num%2 == 0) {
       return true;
    } else {
       return false;}        
} Right boolean isEven(int numberToCheck) {
  // We decouple the what (to check for even or odd)
  // With how (the algorithm)
  return (numberToCheck % 2 == 0);     
} Detection Automatic [x] Many linters can check syntactic trees and look for explicit true/value returns. Tags Primitive Conclusion Search on code libraries for statements and try to replace them when possible. return true Relations Code Smell 36 - Switch/case/elseif/else/if statements Code Smell 118 - Return False More Info How to get rid of IFs forever Credits Photo by on engin akyurt Unsplash The good news is: Anything is possible on your computer. The bad news is: Nothing is easy. - Ted Nelson Software Engineering Great Quotes And that’s all for now… The next article will explain 5 more code smells!

This story contains new, firsthand information uncovered by the writer.

The code in this story is for educational purposes. The readers are solely responsible for whatever they build with it.

A Hacker Tried to Steal $566M  by Exploiting a Code Smell, Here's How

Creating a Wordle with TDD in Javascript

Buy My Book

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

About Author

Comments

TOPICS

THIS ARTICLE WAS FEATURED IN

Related Stories

10 Predictions About the Future of Finance

106 Stories To Learn About Clean Code

100 Pieces of Programming Advice from the Book Clean Code by Robert Martin

110 Stories To Learn About Refactoring

3 Things Coding And Prose Writing Have In Common According To Cory House

4 Simple Habits For Engineering Teams To Beat Technical Debt

10 Predictions About the Future of Finance

106 Stories To Learn About Clean Code

100 Pieces of Programming Advice from the Book Clean Code by Robert Martin

110 Stories To Learn About Refactoring

3 Things Coding And Prose Writing Have In Common According To Cory House

4 Simple Habits For Engineering Teams To Beat Technical Debt

Light-Mode

Classic

Newspaper

Minty

Dark-Mode

Neon Noir

Minty

HN StartUps