DEV Community

Cover image for 5 Bad Practices That Can Make Your C# Code Messy - And How to Avoid Them
Dotnetsafer
Dotnetsafer

Posted on • Updated on • Originally published at blog.dotnetsafer.com

5 Bad Practices That Can Make Your C# Code Messy - And How to Avoid Them

How do you know if you're following good practices when programming in C#? And how can you avoid the bad practices that can make your code messy and harder to manage? In this article, we'll look at five common bad practices, and explain how to avoid them by using some better alternatives.


Stop check null with if statement

The if statement can have an enormous number of utilities and uses. One of them is to use it to check null. Although this is a correct way, it can be stated a little better:

Bad way:

if (application != null)
{
    if (application.protected != null)
    {
        return application.protected.shieldLastRun;
    }
}
Enter fullscreen mode Exit fullscreen mode

Good way:

return application?.protected?.shieldLastRun;
Enter fullscreen mode Exit fullscreen mode

At no point am I saying that you can't use the if statement for null checking, but by having the null conditional (which is what that feature was implemented for), we will get cleaner and easier to read code.

📚Check out the Microsoft article to learn more: Null coalescing operator


Use Tuples instead classes

If you want to return more than one result, the first thing that will probably come to mind is to create a class just for that purpose. This is a correct way to do it but not the best way to do it.

Bad way:

public ApplicationInfo GetInfo()
{
    var application = new ApplicationInfo
    {
        Path = "C:/apps/",
        Name = "Shield.exe"
    };

    return application;
}
Enter fullscreen mode Exit fullscreen mode

Good way:

public (string Path, string Name) GetApplicationInfo()
{
    return ("C:/apps/", "Shield.exe");
}
Enter fullscreen mode Exit fullscreen mode

For this there are tuples, according to Mahesh Chand in his article Tuples in C#:

"Often, we want to return more than one value from a class method. Prior to the introduction of tuples in .NET, there were three common ways to do so.

  • Out parameters
  • Class or struct types
  • Anonymous types returned through a dynamic return type Tuples solve this problem"

As Mahesh Chand says, tuples solve this problem. So using them is a much better option than creating a new class.

📚Check out the Microsoft article to learn more: Tuple types in C#


Avoid modifications by using private members

It is not good practice to have the ability to modify members. You have to be careful which of them can be modified or not.

Bad way:

class Laptop
{
    public string Os{ get; set; } // can be modified
public Laptop(string os)
    {
        Os= os;
    }
}
var laptop = new Laptop("macOs");
Console.WriteLine(Laptop.Os); // Laptop os: macOs
Enter fullscreen mode Exit fullscreen mode

Good way:

class Laptop
{
    public string Os{ get; } // cannot be modified
public Laptop(string os)
    {
        OS = os;
    }
}
var laptop = new Laptop("macOs");
Console.WriteLine(Laptop.Os); // Laptop os: macOs
Enter fullscreen mode Exit fullscreen mode

If the member will not be modified, it is better not to use set; to avoid any accidental (or intentional) modification in the future.

📚Check out the Microsoft article to learn more: Get & Set accessors


Use conditional operator instead if-else

Often out of habit, we get used to using the classic if-else and that's it. I would not really like to consider this as a bad practice but there is a better way to do it:

Bad way:

if (hasApplication)
{
   shouldProtect = true;
}
else
{
   shouldProtect = false;
}
Enter fullscreen mode Exit fullscreen mode

Good way:

bool shouldProtect = hasApplication ? true: false;
Enter fullscreen mode Exit fullscreen mode

As we can clearly see, although it is a similar alternative to if-else, by using the ternary conditional operator ?:, it is much easier to read, understand and we will get a cleaner code.

📚Check out the Microsoft article to learn more: Conditional operator


Avoid initials as identifier abbreviations

When thinking about clean code, one of the first occurrences may be to use initials to shorten the code in identifiers. This is a good practice but you have to be very careful with it.

Bad way:

//different identifiers but same abbreviation
private readonly SecurityManager _sm;
private readonly SoftwareManager _sm;
Enter fullscreen mode Exit fullscreen mode

Good way:

//No confusion
private readonly SecurityManager _securityManager;
private readonly SoftwareManager _softwareManager;
Enter fullscreen mode Exit fullscreen mode

If you are going to use abbreviations, please, always think twice to avoid possible confusion in the future. Many times easy does not equal optimal.

📚Check out the Microsoft article series to learn more: Naming Guidelines

Discussion (10)

Collapse
stegriff profile image
Stephen Griffiths

I get that you're trying to contrive a situation, but I think you should come up with something more in-depth for this:

bool shouldProtect = hasApplication ? true: false;
Enter fullscreen mode Exit fullscreen mode

Because it could just be:

bool shouldProtect = hasApplication
Enter fullscreen mode Exit fullscreen mode

...which means you probably don't need the shouldProtect variable at all...

Collapse
chriz_ profile image
Chriz

The Tuples introduced with C# 7 are not always a better choice; they are value types, which means if your passing back a reference type, its going to 'copied' leave a bigger memory footprint.

There is a reference type version too:

C# tuples, which are backed by System.ValueTuple types, are different from tuples that are represented by System.Tuple types. The main differences are as follows:

-System.ValueTuple types are value types. System.Tuple types are reference types.
-System.ValueTuple types are mutable. System.Tuple types are immutable.
-Data members of System.ValueTuple types are fields. Data members of System.Tuple types are properties.

Collapse
canro91 profile image
Cesar Aguirre • Edited on

Totally agree on tuples...Given that these are guidelines...For 4. we don't actually need a conditional var shouldProtect = hasApplication would be enough I think...

Collapse
cowienduckie profile image
Minh Tran

Small but must-known details for cleaner code 🙆‍♂️🙆‍♂️🙆‍♂️

Collapse
joelbonetr profile image
JoelBonetR

Those are pretty nice things! Sharing it with my C# colleagues, thank you! 😁

Collapse
hamaronooo profile image
hamaronooo

What da hell is last one? XD

Collapse
dimitarbogdanov profile image
Dimitar Bogdanov

what's wrong with it?

Collapse
hamaronooo profile image
hamaronooo

you can't name two variables same names in any csharp dev tool

Thread Thread
dimitarbogdanov profile image
Dimitar Bogdanov

It's obviously an example for ambiguous abbreviations, and not meant to compile.