Post
Ask Question

Common Code Smell Mistakes In C#, Part One

Introduction

A code smell is a word that was popularized by Kent Beck on Words Wiki. It is any characteristic in programming 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. A code smell is a 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 the '+' symbol to concatenate the string in a loop

If you want to concatenate the string in a loop, avoid the usage of the + 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 statements should not be the 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 the 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();
}

An exception should not be rethrown in the 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 a file or database to track the exception. But avoid rethrowing the exception in the 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 a new code smell in my next article. Until then, enjoy coding. I hope this article can make your code better.


Similar Articles