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 Let's continue... Code Smell 101 - Comparison Against Booleans When comparing to booleans, we perform magic castings and get unexpected results. TL;DR: Don't compare against true. Either you are true, or false or you shouldn't compare Problems Hidden castings The least surprise principle violation. principle violation Fail Fast Solutions Use booleans Don't mix booleans with boolean castable objects Context Many languages cast values to boolean crossing domains. Sample Code Wrong #!/bin/bash

if [ false ]; then
    echo "True"
else
    echo "False"
fi

# this evaluates to true since 
# "false" is a non-empty string


if [ false ] = true; then
    echo "True"
else
    echo "False"
fi

# this also evaluates to true Right #!/bin/bash

if  false ; then
    echo "True"
else
    echo "False"
fi

# this evaluates to false Detection Automatic [x] Linters can check for explicit comparisons and warnings. Tags Castings Conclusion It is a common industry practice to use many non-booleans as booleans. We should be very strict when using booleans. Relations Code Smell 69 - Big Bang (JavaScript Ridiculous Castings) More Info Fail Fast Credits Photo by on Michael Held Unsplash If it doesn’t work, it doesn’t matter how fast it doesn’t work. - Mich Ravera Code Smell 102 - Arrow Code Nested IFs and Elses are very hard to read and test TL;DR: Avoid nested IFs. Even Better: Avoid ALL IFs Problems Readability Solutions Extract Method Combine Boolean Conditions Remove accidental IFs Context In procedural code, it is very common to see complex nested ifs. This solution is more related to scripting than object-oriented programming. Sample Code Wrong if (actualIndex < totalItems)
    {
      if (product[actualIndex].Name.Contains("arrow"))
      {
        do
        {
          if (product[actualIndex].price == null)
          {
            // handle no price
          }
          else
          {
            if (!(product[actualIndex].priceIsCurrent()))
            {
              // add price
            }
            else
            {
              if (!hasDiscount)
              {
                // handle discount
              }
              else
              {
                // etc
              }
            }
          }
          actualIndex++;
        }
        while (actualIndex < totalCounf && totalPrice < wallet.money);
      }
      else
        actualIndex++;
    }
    return actualIndex;
  } Right foreach (products as currentProduct)
  addPriceIfDefined(currentProduct)

addPriceIfDefined() 
{
  //Several extracts
} Detection Automatic [x] Since many linters can parse trees, we can check on compile-time for nesting levels. Tags Readability Complexity Conclusion Following , we should leave the code cleaner than we found it. uncle bob's advice Refactoring this problem is easy. Relations Code Smell 78 - Callback Hell Code Smell 03 - Functions Are Too Long Code Smell 36 - Switch/case/elseif/else/if statements More Info C2 Wiki Flattening Arrow Code Refactoring.com The purpose of software engineering is to control complexity, not to create it. - Pamela Zave Code Smell 103 - Double Encapsulation Calling our own accessor methods might seem a good encapsulation idea. But it is not. TL;DR: Don't use setters and getters, even for private use Problems Setters Getters Exposing private attributes Solutions Remove setters Remove getters Protect your attributes Context Using double encapsulation was a standard procedure in the 90s. We wanted to hide implementation details even for private use. This was hiding another smell when too many functions rely on data structure and accidental implementation. For example, we can change an object's internal representation and rely on its external protocol. Cost/benefit is not worth it. Sample Code Wrong contract MessageContract {
    string message = "Let's trade";
    
    function getMessage() public constant returns(string) {
        return message;
    }
    
    function setMessage(string newMessage) public {
        message = newMessage;
    }
    
    function sendMessage() public constant {
        this.send(this.getMessage());
        // We can access property but make a self call instead
    }
} Right contract MessageContract {
    string message = "Let's trade";
        
    function sendMessage() public constant {
        this.send(message);
    }
} Detection Semi-Automatic [x] We can infer getters and setters and check if they are invoked from the same object. Tags Encapsulation Conclusion Double encapsulation was a trendy idea to protect accidental implementation, but it exposed more than protected. Relations Code Smell 37 - Protected Attributes Code Smell 28 - Setters Code Smell 68 - Getters More Info Stack Exchange Reddit InfoWorld Credits Photo by on Ray Hennessy Unsplash Encapsulate the concept that varies. - Erich Gamma Code Smell 104 - Assert True Asserting against booleans makes error tracking more difficult. TL;DR: Don't assert true unless you are checking a boolean Problems Fail Fast Principle Solutions Check if the boolean condition can be rewritten better Favor assertEquals Context When asserting a boolean our test engines cannot help us very much. They just tell us something failed. Error tracking gets more difficult. Sample Code Wrong <?

final class RangeUnitTest extends TestCase {
 
  function testValidOffset() {
    $range = new Range(1, 1);
    $offset = $range->offset();
    $this->assertTrue(10 == $offset);    
    // No functional essential description :(
    // Accidental description provided by tests is very bad
  }  
}

// When failing Unit framework will show us
//
// 1 Test, 1 failed
// Failing asserting true matches expected false :(
// () <-- no business description :(
//
// <Click to see difference> - Two booleans
// (and a diff comparator will show us two booleans) Right <?

final class RangeUnitTest extends TestCase {
 
  function testValidOffset() {
    $range = new Range(1, 1);
    $offset = $range->offset();
    $this->assertEquals(10, $offset, 'All pages must have 10 as offset');    
    // Expected value should always be first argument
    // We add a functional essential description
    // to complement accidental description provided by tests
  }  
}

// When failing Unit framework will show us
//
// 1 Test, 1 failed
// Failing asserting 0 matches expected 10
// All pages must have 10 as offset <-- business description
//
// <Click to see difference> 
// (and a diff comparator will help us and it will be a great help
// for complex objects like objects or jsons) Detection Semi-Automatic [x] Some linters warn us if we are checking against boolean after setting this condition. We need to change it to a more specific check. Tags Test Smells Conclusion Try to rewrite your boolean assertions and you will fix the failures much faster. Relations Code Smell 101 - Comparison Against Booleans Code Smell 07 - Boolean Variables More Info Fail Fast Credits Photo by on Joël de Vriend Unsplash I've finally learned what 'upward compatible' means. It means we get to keep all our old mistakes. - Dennie van Tassel Code Smell 105 - Comedian Methods Use professional and meaningful names TL;DR: Don't be informal or offensive Problems Readability Unprofessional work Solutions Choose good and professional names. Context Our profession has a creative side. Sometimes we get bored and try to be funny. Sample Code Wrong function erradicateAndMurderAllCustomers();

// unprofessional and offensive Right function deleteAllCustomers();

// more declarative and professional Detection Semi-Automatic [x] We can have a list of forbidden words. We can also check them in code reviews. Names are contextual, so it would be a difficult task for an automatic linter. Naming conventions should be generic and should not include cultural jargon. Tags Naming Conclusion Be professional in the way you name things in your code. Don't be trying to be a comedian by giving a variable a silly name. You should write production code so future software developers (even you) should easily understand. Relations Code Smell 38 - Abstract Names More Info What is in a name Credits Photo by on Stewart Munro Unsplash This ‘users are idiots, and are confused by functionality’ mentality of Gnome is a disease. If you think your users are idiots, only idiots will use it. - Linus Torvalds Software Engineering Great Quotes And that’s all for now… The next article will explain 5 more code smells!

Keep

Linux Explained on 5 Levels of Difficulty

5 Ways to Prevent Server Hardware Failure

Buy My Book

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

About Author

Comments

TOPICS

THIS ARTICLE WAS FEATURED IN

Related Stories

Untitled Story

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

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

Light-Mode

Classic

Newspaper

Dark-Mode

Neon Noir

Minty

HN StartUps