Common Code Smell Mistakes In C#, Part One

Code smell is a word given to indicate a deeper problem in our programming code. Here, we will discuss about some of the code smell vulnerabilities that developers commonly face but don't recognize sometimes.

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 
  1. bool isTrue= true;  
  2.   
  3. if(isTrue == true)  
  4. {  
  5.       // Code  
  6. }  
  7.   
  8. if(isTrue == false)  
  9. {  
  10.      // code  
  11. }  
  12.   
  13. bool IsBoolMethod()  
  14. {  
  15.     return true;  
  16. }  
  17.   
  18. if(IsBoolMethod () == false)  
  19. {  
  20.      // code  
  21. }  
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 
  1. bool isTrue= true;    
  2.     
  3. if(isTrue)    
  4. {    
  5.       // Code    
  6. }    
  7.     
  8. if(! isTrue)    
  9. {    
  10.      // code    
  11. }    
  12.     
  13. bool IsBoolMethod()    
  14. {    
  15.     return true;    
  16. }    
  17.     
  18. if( ! IsBoolMethod ())    
  19. {    
  20.      // code    
  21. }   

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.
  1. try  
  2. {  
  3.      //Code  
  4. }  
  5. catch (Exception ex)  
  6. {  
  7.     throw ex; // Non Compliant Code  
  8. }  
  9.   
  10. // Compliant Code  
  11. try  
  12. {  
  13.     // code  
  14. }  
  15. catch(Exception ex)  
  16. {  
  17.    throw;  
  18. }  

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. 
  1. // Do not use this  
  2. string str = "";  
  3. for (int i = 0; i < 10 ; i++)  
  4. {  
  5.   str = str + i + "-";  // Non Compliant code  
  6. }  
  7.   
  8. // Instead use this  
  9. StringBuilder sb = new StringBuilder();  
  10. for (int i = 0; i < 10 ; i++)  
  11. {  
  12.   sb.Append(i).Append("-")  
  13. }  
  14. 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. 
  1. // Non Compliant code  
  2. if (num < 0)  
  3. {  
  4.   DoFirst();  
  5.   DoTheThing();  
  6. }  
  7. else if (num > 1 && num < 3)  
  8. {  
  9.   DoFirst();  // Duplicate code  
  10.   DoTheThing(); // Duplicate code  
  11. }  
  12. else  
  13. {  
  14.   DoSomething();  
  15. }  
  16.   
  17.   
  18. // Compliant code  
  19.   
  20. if (num < 0 || (num > 1 && num < 3) ) 
  21. {  
  22.   DoFirst();  
  23.   DoTheThing();  
  24. }  
  25. else  
  26. {  
  27.   DoSomething();  
  28. }   

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.
  1. // Non compliant code  
  2. try  
  3. {  
  4.   // Code that might have exception  
  5. }  
  6. finally  
  7. {  
  8.    throw new Exception("Error in finally");  
  9. }  
  10.   
  11.   
  12. // Compliant code  
  13. try  
  14. {  
  15.   // Code that might have exception  
  16. }  
  17. finally  
  18. {  
  19.    //Clean up or write the code to log the exception in file or database  
  20. }   
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.