Real World Coding Issues: Part 2 – Style and Performance Issues

In Part 1 of this series of articles about real-world coding issues, I highlighted the code violations I discovered in the contract I worked on. The codebase exhibited over 125K instances of violations, and I provided statistical data to support this finding. Additionally, I delved into other problems such as cyclomatic complexity, memory issues, and performance improvements. If you have not done so already, I encourage you to read Part 1 by clicking here: https://bit.ly/RealWorldCodingIssues1

An enhanced edition of this article can be found in my book, “Rock Your Code: Coding Standards for Microsoft .NET,” now available on Amazon. This updated version comprises 230 new pages, providing valuable insights to help you and your team produce top-quality code that is easily modifiable.

Secure your copy by visiting the following link: https://bit.ly/CodingStandards8

In this article, I will discuss common coding issues that I frequently encounter in the projects I work on. These issues fall into the “style” category of code analyzers, but it is worth noting that many of them can also impact code performance, an area in which I have expertise.

Remove Unnecessary Casts

Type casting converts one type to another. Unnecessary typecasting not only makes the code less clean, but it can also cause performance issues. Here is an example that demonstrates this problem:

return new CSDateTime(csDate.AddMinutes((double)units), this.DateType);

In this code example, csDate is a DateTime and units is an integer. In this case, an integer can be easily used in a parameter type that is a double. To fix it, just remove the cast.

return new CSDateTime(csDate.AddMinutes(units), this.DateType);

For more information and the EditorConfig analysis setting, go to the complete article:

Microsoft .NET Code Analysis: Remove Unnecessary Casts

Remove Unnecessary Using Directives

I consistently remove unnecessary “using” directives for several reasons, which I extensively discuss in my coding standards book and conference sessions. Firstly, let me demonstrate the problem:

using log4net;
using System.Globalization;
using System.Diagnostics; // < No longer in use

In this scenario, the “System.Diagnostics” namespace is no longer utilized, hence the appropriate solution is to remove it. To ensure cleanliness and proper organization, you can utilize tools such as CodeRush or the code cleanup fixes available in Visual Studio to remove unnecessary using directives during a file save.

For more information and the EditorConfig analysis setting, go to the complete article:

Microsoft .NET Code Analysis: Remove Unnecessary Using Directives

Always Add Braces in C#

Not adding braces {} to if statements can lead to issues and affect code readability. The following code demonstrates a violation of this practice:

if (somelist[i] != other.somelist[i])
    return false;

This is how you fix it:

if (somelist[i] != other.somelist[i])
{
    return false;
}

It is crucial to always include braces when writing if statements. For more information and the EditorConfig analysis setting, go to the complete article:

Microsoft .NET Code Analysis: Always Add Braces {} in C#

Inline Variable Declaration

Starting with .NET 7, it is recommended to inline the variable declaration when calling a method with an out parameter. To illustrate the issue, consider the following example:

double number1;
if(double.TryParse(dollarAmount, out number1))

To fix it, just move the declaration to the parameter as shown below:

if(double.TryParse(dollarAmount, out double number1))

Using inline variable declarations can improve the performance of the code. For more information and the EditorConfig analysis setting, go to the complete article:

Microsoft .NET Code Analysis: Inline Variable Declaration

Use Pattern Matching to Avoid ‘As’ Followed by A ‘Null’ Check

The “is” keyword was introduced in the .NET Framework 2.0 and is considered the recommended approach for avoiding null checks on objects. Here is an example of the issue:

var other = obj as CustomArgumentInstance;
if ((object)other == null)
{
    return false;
}

This is how you fix the code violation:

if (obj is not CustomArgumentInstance other)
{
    return false;
}

The code fix not only improves the cleanliness of your code but also has the potential to enhance performance. By utilizing pattern matching, type checking and extraction are combined, resulting in more efficient execution.

For more information and the EditorConfig analysis setting, go to the complete article:

Microsoft .NET Code Analysis: Use Pattern Matching to Avoid ‘As’ Followed by A ‘Null’ Check

Always Use Accessibility Modifiers

Accessibility modifiers are essential in object-oriented programming as they control the visibility of types and members. During my analysis of a codebase for this article, I discovered an unprecedented number of missing modifiers. This prompted me to discuss the importance of using accessibility modifiers in my coding standards book. It is crucial to be mindful of their usage.

Here are my recommended guidelines:

1. Most classes should be marked as internal, indicating that they are accessible only within the same assembly. Public access should be limited to classes intended for reuse by other assemblies or if a specific framework necessitates it.
2. All fields should be marked as private to enforce encapsulation in object-oriented programming. By restricting direct access to fields, you ensure proper data encapsulation and maintain control over their usage.
3. For methods, properties, delegates, and events, it is advisable to mark them as private or internal in most cases. These members should only be made public if they are explicitly designed to be called by another assembly.

For more information and the EditorConfig analysis setting, go to the complete article:

Microsoft .NET Code Analysis: Always Use Accessibility Modifiers

Remove Unread Private Members

When code is modified or removed within a class, it is common for unused private members, such as variables, methods, and properties, to remain. Removing these unused private members is essential for enhancing code readability and facilitating maintenance.

Frequently, I come across a situation where a field is initialized in a constructor but remains unused within the class. This issue was prevalent in the code base I worked with for this article. Neglecting to address this oversight can have performance implications, particularly depending on the nature of the variable involved.

For more information and the EditorConfig analysis setting, go to the complete article:

Microsoft .NET Code Analysis: Remove Unread Private Members

The Importance of the readonly Modifier

The readonly modifier is utilized to indicate fields or variables that can only be assigned once, typically during object creation or in the constructor. It is commonly employed in constructors for variables received from the calling code, such as database connections, to ensure their immutability throughout the object’s lifespan. Proper usage of the readonly modifier is crucial, and during my review of the code for this article, I noticed excessive instances where it was missing.

For more information and the EditorConfig analysis setting, go to the complete article:

Microsoft .NET Code Analysis: The Importance of the readonly Modifier

Use Compound Assignments

Compound assignments have been available since the release of .NET and are the preferred method for modifying numbers and strings. Here is an example illustrating the issue:

var peopleCount = 0;
var loopedCount = 0;
foreach (var peoplePage in people.Page(10))
{
	loopedCount++;
	peopleCount = peopleCount + peoplePage.Count();
}

Here is how to fix it:

foreach (var peoplePage in people.Page(10))
{
	loopedCount++;
	peopleCount += peoplePage.Count();
}

For more information and the EditorConfig analysis setting, go to the complete article:

Microsoft .NET Code Analysis: Use Compound Assignments

Use the Index Operator

C# 8 introduced the index-from-end operator for accessing items in a collection. Prior to C# 8, here is an example of how you would do it:

return results[results.Count - 1];

In this case, this is how this would be improved by using the index operator:

return results[^1];

For more information and the EditorConfig analysis setting, go to the complete article:

Microsoft .NET Code Analysis: Use the Index Operator

Remove Unnecessary Expression Values

When coding, it is crucial to pay attention to the value returned by a method and avoid ignoring or discarding it. Neglecting the return value is not just a matter of style; it has implications that go beyond code readability and cleanliness.

My concerns are performance, introducing confusion during the debugging process and making maintenance more challenging. This is an example of the issue:

_memoryCache.Set(key, obj, new TimeSpan(0, minutes, 30));

The _memoryCache is an instance of IMemoryCache. To fix, use a discard as shown below:

_ = _memoryCache.Set(key, obj, new TimeSpan(0, minutes, 30));

Discards, similar to unassigned variables, do not hold a value. However, utilizing discards can serve as a communication tool for both the compiler and other developers who read your code. It explicitly expresses your intention to disregard the result of an expression. Incorporating discards can be a beneficial practice.

For more information and the EditorConfig analysis setting, go to the complete article:

Microsoft .NET Code Analysis: Remove Unnecessary Expression Values

Remove Unnecessary Value Assignments

For optimal performance, it is crucial to eliminate unnecessary value assignments. Consider the following example to illustrate the problem:

List<People> users = null;
int count = 0;

The default value for reference types is null, and for integers, it is zero. Therefore, there is no need to explicitly set these values in your code. To address this, simply remove the assignment, as demonstrated below:

List<People> users;
int count;

Only assign a value if it is different from the default value. This cleanup can be performed by CodeRush during the saving of a file along with one of the code cleanup fixes in Visual Studio.

For more information and the EditorConfig analysis setting, go to the complete article:

Microsoft .NET Code Analysis: Remove Unnecessary Value Assignments

Remove Unused Parameters

Unused parameters in methods can lead to various issues and should be consistently removed. During the code review for this article, I encountered a significant number of such problems, requiring a considerable amount of time to fix. To demonstrate this, let’s examine the following example:

public OrderController(ILogger<OrderController> logger, IRouter router)
{
    _webRouter = router;
}

As you can see, the parameter “logger” is not used, so to fix it just remove it!

public OrderController(IRouter router)
{
    _webRouter = router;
}

To address this issue, I utilize CodeRush, which not only eliminates the unused parameter but also refactors the code in all instances where the method is invoked. This makes the process straightforward and efficient.

For more information and the EditorConfig analysis setting, go to the complete article:

Microsoft .NET Code Analysis: Remove Unused Parameters

Proper Using Directive Placement

In .NET, there are multiple locations where you can include using directives in a code file. According to coding standards, it is recommended to place them outside of the namespace. Here is an example of how I place using directives in my Spargine OSS project.

One of the main reasons for placing the using directive outside of the namespace is to facilitate easier refactoring. For more information and the EditorConfig analysis setting, go to the complete article:

Microsoft .NET Code Analysis: Proper Using Directive Placement

Use the Switch Expressions Instead of Statements

Switch expressions were introduced in .NET Core 3 and have quickly become the recommended approach for implementing switch statements, thanks to their numerous advantages. Let’s consider the following example of a switch statement:

public enum DayOfWeek
{
    Monday,
    Tuesday,
    Wednesday,
    Thursday,
    Friday,
    Saturday,
    Sunday
}
public string GetDayOfWeekName(DayOfWeek dayOfWeek)
{
    string dayName;
    
    switch (dayOfWeek)
    {
        case DayOfWeek.Monday:
            dayName = "Monday";
            break;
        case DayOfWeek.Tuesday:
            dayName = "Tuesday";
            break;
        case DayOfWeek.Wednesday:
            dayName = "Wednesday";
            break;
        case DayOfWeek.Thursday:
            dayName = "Thursday";
            break;
        case DayOfWeek.Friday:
            dayName = "Friday";
            break;
        case DayOfWeek.Saturday:
            dayName = "Saturday";
            break;
        case DayOfWeek.Sunday:
            dayName = "Sunday";
            break;
        default:
            dayName = "Invalid day";
            break;
    }
    
    return dayName;
}

While the conventional switch statement syntax has been widely used by developers since the beginning of .NET, a more modern and concise approach is available through switch expressions. By embracing switch expressions, we can refactor the code as shown below:

public string GetDayOfWeekName(DayOfWeek dayOfWeek)
{
    string dayName = dayOfWeek switch
    {
        DayOfWeek.Monday => "Monday",
        DayOfWeek.Tuesday => "Tuesday",
        DayOfWeek.Wednesday => "Wednesday",
        DayOfWeek.Thursday => "Thursday",
        DayOfWeek.Friday => "Friday",
        DayOfWeek.Saturday => "Saturday",
        DayOfWeek.Sunday => "Sunday",
        _ => "Invalid day"
    };
    
    return dayName;
}

This approach is significantly cleaner and more streamlined. One notable advantage of switch expressions is the elimination of the need to add break statements. In the past, it was common to overlook or accidentally omit breaks in switch statements, leading to build errors. However, with switch expressions, this concern is eliminated.

For more information and the EditorConfig analysis setting, go to the complete article:

Microsoft .NET Code Analysis: Use the Switch Expressions Instead of Statements

Use Pattern Matching

Pattern matching was introduced in the C# programming language with the release of C# 7 and has since become the preferred approach for checking the shape or structure of data. Since its inception, pattern matching has gained widespread usage in coding. Here is an example of code written prior to the introduction of pattern matching in C#:

if (result.ObjectType == ProjectTypes.Boolean || 
  result.ObjectType == ProjectTypes.String)
{
  return (result.ToString() == "true");
}

Here is how you would refactor this using pattern matching:

if (result.ObjectType is ProjectTypes.Boolean or ProjectTypes.String)
{
  return (result.ToString() == "true");
}

Pattern matching is much cleaner and easier to understand. For more information and the EditorConfig analysis setting, go to the complete article:

Microsoft .NET Code Analysis: Use Pattern Matching

Summary

In this article, I have highlighted fifteen common style issues that can also have an impact on performance. The forthcoming articles will provide in-depth analysis of additional common issues, complemented by comprehensive code examples and detailed explanations. It is worth noting that most of the issues discussed in this article can be easily addressed by utilizing the Visual Studio extension called CodeRush. CodeRush is a free tool available for download at the following link: https://www.devexpress.com/products/coderush/

For further guidance and insights, I highly recommend obtaining a copy of my book, “Rock Your Code: Coding Standards for Microsoft .NET” available on Amazon.com. Additionally, to explore more performance tips for .NET, I encourage you to acquire the 3rd edition of “Rock Your Code: Code & App Performance for Microsoft .NET” also available on Amazon.com.

To analyze your code using the same settings I used in these articles, I encourage you to incorporate my EditorConfig file. It can be found at the following link: https://bit.ly/dotNetDaveEditorConfig. I update this file quarterly, so remember to keep yours up to date as well. I hope you will check out my OSS project Spargine by using this link: https://bit.ly/Spargine.

Please feel free to leave a comment below. I would appreciate hearing your thoughts and feedback.

Pick up any books by David McCarter by going to Amazon.com: http://bit.ly/RockYourCodeBooks

If you liked this article, please buy David a cup of Coffee by going here: https://www.buymeacoffee.com/dotnetdave

© The information in this article is copywritten and cannot be preproduced in any way without express permission from David McCarter.