Common Code Smell Mistakes In C#, Part One

Introduction

 

Code smell is a word which was popularized by Kent Beck on words wiki. It is any characteristic in programing source code that indicates a  deeper problem in our code.

 

Code smell is not a bug but it is a sign of bad construction of your program. Code smell is language and application-independent; this means we can have code smell in any language and any application.

 

We can find code smells in our code in different ways. Some of them are listed below.

• Code Inspection
• Refactoring of the code
• Heuristics analysis
• Third-party tools such as ReSharper (I am a big fan of this tool; it improved my coding technique a lot), Sonarqube, etc.

Here, we will discuss some common code smells that developers do while developing their code.

 

Redundant boolean literal

 

This is one of the common mistakes developers make. Let's discuss this with an example.

 

Non-Compliant Code 

bool isTrue= true;

if(isTrue == true)
{
      // Code
}

if(isTrue == false)
{
     // code
}

bool IsBoolMethod()
{
    return true;
}

if(IsBoolMethod () == false)
{
     // code
}

Here, in the above code, there is no need to write the `true` or `false` literals because already the variable isTrue is of boolean type. We can replace the above code as below.

 

Compliant Code 

bool isTrue= true;

if(isTrue)
{
      // Code
}

if(! isTrue)
{
     // code
}

bool IsBoolMethod()
{
    return true;
}

if( ! IsBoolMethod ())
{
     // code
}

Explicitly throwing the exceptions

 

When we are rethrowing an exception, we should do it by simply calling throw; and not throw ex;, because the stack trace is reset with the second syntax, which makes debugging complex.

try
{
     //Code
}
catch (Exception ex)
{
    throw ex; // Non Compliant Code
}

// Compliant Code
try
{
    // code
}
catch(Exception ex)
{
   throw;
}

Avoid usages of '+' symbol to concatenate the string in a loop

 

If you want to concatenate the string in a loop, avoid the usage of + symbol to concatenate the string. Instead, use the StringBuilder to append it to a string. StringBuilder is more efficient than string concatenation when the operator is iterated in a  loop. 

// Do not use this
string str = "";
for (int i = 0; i < 10 ; i++)
{
  str = str + i + "-";  // Non Compliant code
}

// Instead use this
StringBuilder sb = new StringBuilder();
for (int i = 0; i < 10 ; i++)
{
  sb.Append(i).Append("-")
}
string str= sb.ToString();

Implementations of conditional statement should not be same

 

Two branches in the same conditional structure should not have the same implementation. If the same logic is required for both branches, it is better to combine the statements.

 

As shown in the below example, both if and else if have same code block; so we can merge both blocks as shown in the compliant solution below. 

// Non Compliant code
if (num < 0)
{
  DoFirst();
  DoTheThing();
}
else if (num > 1 && num < 3)
{
  DoFirst();  // Duplicate code
  DoTheThing(); // Duplicate code
}
else
{
  DoSomething();
}


// Compliant code

if (num < 0 || (num > 1 && num < 3) )
{
  DoFirst();
  DoTheThing();
}
else
{
  DoSomething();
}

Exception should not be rethrown in finally block

 

If you are using a try-catch block in the finally block, avoid rethrowing the exception. You can clean up the exception or log the exception in file or database to track the exception. But avoid rethrowing the exception in finally block.

// Non compliant code
try
{
  // Code that might have exception
}
finally
{
   throw new Exception("Error in finally");
}


// Compliant code
try
{
  // Code that might have exception
}
finally
{
   //Clean up or write the code to log the exception in file or database
}

That's it for today. I'll come up with new code smell in my next article. Until then, enjoy coding. I hope this article can make your code better.