Anthony Chu

I think it's really important to write readable code. Readable code is easier to use, maintain, and reason about. It is also more difficult to create bugs in code that is readable.

I believe that code readability is more important than things like performance. That's not to say I don't care about performance, but I'd rather start with readable code and optimize where needed. A lot of readability problems in code often come from premature optimization.

Sometimes we write unreadable code simply because we don't know better. So I thought in this article I'd go through a few specific examples of how to make .NET code more readable. They're examples I've come across in code that I have written or reviewed.

TimeSpan Factory Methods

.NET's TimeSpan struct has four constructor overloads that take one, three, four, and five integer parameters. It's not uncommon to come across code like this:

var duration1 = new TimeSpan(1, 0, 0);
var duration2 = new TimeSpan(0, 1, 0, 0);

Both of these statements create a TimeSpan of 1 hour. But unless you have memorized the different constructor overloads or use IntelliSense, it is near impossible to figure out what the actual value is by reading it.

One might say we can solve this by giving the variables a proper name. I think it's incredibly important to give variables meaningful and descriptive names, but that can only get us so far. Take this example:

var oneHour = new TimeSpan(0, 0, 1, 0, 0);

What's wrong with this? Well, it turns out the signature of this particular overload is public TimeSpan(int days, int hours, int minutes, int seconds, int milliseconds); so we have a TimeSpan of 1 minute, not 1 hour. Whoops!

A better way to solve this is using TimeSpan's static factory methods:

var oneHour = TimeSpan.FromHours(1);
var fiveMinutes = TimeSpan.FromMinutes(5);

String.Contains()

String.IndexOf() is often used to find out if a string contains another string:

if (foo.IndexOf("bar") >= 0)
{
    // ...
}

It's easy to make a mistake with the comparison operator. It's much better to use the String.Contains() method. It improves readability too:

if (foo.Contains("bar"))
{
    // ...
}

Named Parameters

Quite often we'll come across a method that has one or more boolean parameters that trigger different logic inside. Something like this:

public void CreateOrder(Order order, bool sendEmailToCustomer, bool sendEmailToSalesRep)
{
    // ...
}

This makes the calling code difficult to understand:

CreateOrder(order, true, false);

I would argue that more often than not, this is a code smell that perhaps the method is doing too much. But ignoring that, we can improve readability by using C# named parameters when calling the method:

CreateOrder(order, sendEmailToCustomer: true, sendEmailToSalesRep: false);

Duplicating Framework APIs

Sometimes we come across code that is technically correct, but there's something built into the framework that already implements the logic. Here's an example:

HttpContext.Current.Response.StatusCode = 301;
HttpContext.Current.Response.StatusDescription = "301 Moved Permanently";
HttpContext.Current.Response.RedirectLocation = url;
HttpContext.Current.Response.End();

There's nothing technically wrong with what's there, but HttpResponse actually has a RedirectPermanent() method that does this in one line:

HttpContext.Current.Response.RedirectPermanent(url);

API Symmetry

I believe we should all write code as if we're building libraries that'll be distributed outside our current project or solution. One positive side effect of this approach is we end up with classes and interfaces that have a clean public API surface. Classes that have clean APIs are easier to use and maintain; and they lead to more readable code.

For an example of what I believe to be a poor API, consider this interface for encoding and decoding values:

public interface IStringEncoder
{
    byte[] Encode(string value);
    string ReverseEncoding(string encodedValue);
}

There are a couple of issues here; and they both relate to symmetry. When an API is well-designed, methods that perform opposite functions should be symmetric.

The first problem with the above example is the method names. Because the methods perform opposite actions, they should have symmetric names. For example: Encode() and Decode().

The second problem with this API is the asymmetry between the parameter and return type of each method. As an API author, we need to decide if an encoded value should be a string or a byte array, and be consistent in the two methods.

Here's what the interface would look like if it were symmetrical:

public interface IStringEncoder
{
    byte[] Encode(string decodedValue);
    string Decode(byte[] encodedValue);
}