C# Best Practices to get through your next Code Review

C# Best Practices to get through your next Code Review

C# has evolved a lot in recent years with much more efficient and logical ways of doing things than before. In this article, I am going to share some of the best practices for writing cleaner C# code which will help in your next code review and also raise fewer issues in your Sonar builds.

Before we start, I would like to mention that these practices I have incorporated over my 6+ years of experience working as a Dot Net developer.

Utilizing Extension Methods for better Error Handling

Extension methods are probably one of my most used features in C#. I tend to create various extension methods for scenarios to deal with errors, do dependency injection, etc. One common use case and best practice are to create a method to check for null on your collections.

The below example can be used as a reference

public static void Main(string[] args)
{
   try
     {
       var importName = GetNamesFromThirdPartyAPI();
       importName.EmptyIfNull().ForEach(x =>  // will break here
         {
            Console.WriteLine(x);
         });
            Console.WriteLine("Hello World!");
      }

   catch(Exception ex)
   {
       Console.WriteLine(ex.Message);
   }
   finally {
      Console.ReadLine();
   }
}

private static IEnumerable<string> GetNamesFromThirdPartyAPI()
{
    //can be any third party method, DB call, or an API call
    return null;
}

Now if we don't check for null, our solution is going to break, or every time check if the variable is null, and if not then continue. The easier and more reusable way would be to create an extension method like

public static IEnumerable<T> EmptyIfNull<T> (this IEnumerable<T> data)
{
   return data ?? Enumerable.Empty<T>();
}

What it will basically do is return an empty new List every time it's null. We can then use it on all IEnumerables in our code directly.

importName.EmptyIfNull().ToList().ForEach(x =>
{
   Console.WriteLine(x);
});

Handling Boolean variables in Conditions

Sometimes there is a lot of redundancy in our code when we check boolean values against true and false and then perform some task. Instead, they can by themselves be used as the output of the condition as they are boolean reducing the lines of code

So instead of

var items = GetAllItems();
if(items.Any() == true)
{
  ShowDetails.Visible = true;
}
else
{
  ShowDetails.Visible = false;
}

we should be doing

var items = GetAllItems();
if(items.Any())
{
 ShowDetails.Visible = true;
}
else
{
  ShowDetails.Visible = false;
}

even better would be

var items = GetAllItems();
ShowDetails.Visible = items.Any();

String Comparison

Instead of directly comparing two strings with the equality check operator == one should use the .Equals() property as it exposes much more efficient configurations that can help perform accurate comparisons like the case, culture, and sort rules. These ns the traditional comparison way will have to be manually handled by converting both strings to lower case and then performing the comparison.

Empty Check on String

Very often in our codebase, we check if a string variable is empty and perform some logic accordingly. One thing to take care of with string is that the null check should also be done since it is a reference type in C#.

Now if we follow the traditional way of checking it would be something like below:

string shortName = GetShortNameBasedOnId(itemId);
if(shortName != null && shortName != "")
{
    //do something here
}

With IsNullOrWhiteSpace all these checks can be better handled at once, as it indicates whether a specified string is null, empty or consists of white-spaces characters only.

string shortName = GetShortNameBasedOnId(itemId);
if(!string.isNullOrWhiteSpace(shortName) //do something

Safe Check on Properties using ?

Whenever we get some data back from an external source we usually format the data into view models so that it fits our requirements. There can be situations when say some of the properties are null and if we try to read values from them our code is going to again break. An efficient way of handling these is using the ? operator and making sure that the values on the left are not null for the values on the right we are trying to read.

Let's update the return method in our previous code to understand this by an example.

private static List<Offer> GetNamesFromThirdPartyAPI()
        {
            Store store = null;
            return new List<Offer>()
            {
                new Offer()
                {
                    OfferName = "New Year Offer",
                    Details = "New year bonanza offer",
                    ApplicableStore = store
                }
            };
        }

Here the offer object will be containing the values but the store object is null. Now if we directly read this it will break our code, or we can keep using the old if(store !=null) way. However, the smarter way would be to use the following

 var importName = GetNamesFromThirdPartyAPI();
                importName.EmptyIfNull().ToList().ForEach(x =>
                {
                    Console.WriteLine(x.OfferName);
                    Console.WriteLine(x.Details);
                    Console.WriteLine(x.ApplicableStore?.StoreName);

                });

Not Awaiting Every Async Calls Immediately

Another thing that I have observed with some of my teammates is that most often than not they always await all the async calls they make. Which literally removes the asynchronous behavior and makes it completely synchronous.

The concept is that we make an async call and let it run, continue other logic, and then finally await on that async call when we actually need the returning value from it. Let us understand this with a very basic example of making breakfast.

What we want to do is put the tea on the stove and let it boil and in the meanwhile toast the bread and then butter it. Now the tea part here is our async operation which will take time. Let's represent it using Task.Delay() so that it takes some time to complete.

public static async Task<string> TimeTakingTask()
        {
            Console.WriteLine("tea is being prepared");
            await Task.Delay(5000);
            return "Tea is ready";
        }

Now the wrong way of doing this would be to do the below. Wrong because we are awaiting then and there for the tea. Putting the await stops us from going forward. Meaning we will be waiting for the tea to be made and after that, we will Toast the bread and then apply butter.

 var tea =  await TimeTakingTask();
 Console.WriteLine("Toasting Bread");
 Console.WriteLine("Applying Butter");
 Console.WriteLine( tea);``

The correct way of actually achieving asynchronous behavior here would be to let the tea cook and finally put an await on it when we are done with everything else.

var tea =   TimeTakingTask();
Console.WriteLine("Toasting Bread");
Console.WriteLine("Applying Butter");
Console.WriteLine( await tea);

String Interpolation using $

Even though it's quite old now but still many developers use the traditional way of concating string either by using + or the risky string.Format.

The benefit of using the $ is that firstly it's the latest standard across various languages and secondly that it is far more readable and safe in terms of chances of errors.

string name = "Hashnode";
string title = "C# best practices";
string author = "Me myself...";

var oldestWay = string.Format("Blog has been written on {0}, with the title {1} by {2}.", name, title, author);
var oldWay = "Blog has been written on " + name + ", " + "with the title " + title + " " + "by " + author + ".";
var mostReadableWay = $"Blog has been written on {name}, with the title {title} by {author}.";

Console.WriteLine(oldestWay);
Console.WriteLine(oldWay);
Console.WriteLine(mostReadableWay);

Some Additional One-Liners Tip

  • Use .Any instead of Count
  • Avoid declaring variable if it's just used to set a value and then return it from a method, instead directly return the value
  • Mostly 99.9% cases of dependency injection are achieved using the construction injection pattern. Try avoiding the other two ways.
  • Use proper naming convention inside the lambda function instead of the common x
  • Using ternary operator instead of the standard if else as it drastically reduces the size of the code block
  • My SonarLint build once failed as I had more than 8 parameters in a method. Ideally, it is suggested that if there are that many parameters, it's better to create a model and pass it instead of passing on the individual fields.

GIF

It's always nice to do things efficiently and I hope some of these tips might have been new for you. Till next time, keep learning and keep building.

Did you find this article valuable?

Support Rajat Srivastava by becoming a sponsor. Any amount is appreciated!