Clean Code – Is my code always readable?

Did you ever have to go back to a fragment of code that you wrote a month or year ago? How did it feel? Was it easy or did you have to figure out how it worked from scratch? If you need more than just one look, there is a good chance that you are doing something wrong. And if you scratch your head and think: “What the heck was I thinking?”, you have definitely done it wrong. But what has gone wrong? Most probably the code works fine and at some point you knew it inside out. Why can’t you remember it now? Maybe your code wasn’t written clearly enough and in accordance with best coding practices? Here are a few tips on how to write easily readable code not just for yourself but also for other developers.

Example of using coding standards

Consider the following method in C#:

public string transform(List<DateTime> s)
{string d = null;
foreach (DateTime kc in s) 
{if (kc > DateTime.Now) 
{ d = d + kc + "\n"; } 
else { d = d + "Delayed\n"; }}
return d;}

At first glance you have no idea what it actually does or what it can be used for. But after short refactoring we can get:

public string GetText(List<DateTime> arrivalTimes)
{
    var stringBuilder = new StringBuilder();
    foreach (DateTime arrivalTime in arrivalTimes) 
    {
        if (arrivalTime > DateTime.Now) 
        {
            stringBuilder.AppendLine(arrivalTime.ToString());
        }
        else 
        {
            stringBuilder.AppendLine("Delayed");
        } 
    }
    return stringBuilder.ToString();
}

Or if we apply the “?:” operator, we will get:

public string GetText(List<DateTime> arrivalTimes)
{
    var stringBuilder = new StringBuilder();
    foreach (DateTime arrivalTime in arrivalTimes)
    {
        string message = arrivalTime > DateTime.Now ? arrivalTime.ToString() : "Delayed";
        stringBuilder.AppendLine(message);
    }
    return stringBuilder.ToString();
}

What has actually happened to the code? Several modifications have been made to increase its readability:

1. The name of the method has been changed from one that really doesn’t say anything to one that is a little bit more descriptive.
2. The way of naming the variables was changed:
   1. “kc” was changed to arrivalTime,
   2. “s” was changed to arrivalTimes,
   3. “d” was changed to stringBuilder,

It also much easier to understand what each variable is responsible for and how it was used.

3. Parentheses have been standardized to one format.
4. Tabs have been added to increase readability, spacing, and nesting in braces.
5. The “?:” operator has been used to reduce the length of the code by 4 lines.
6. The StringBuilder class was added to avoid string concatenation  (“string” + “string”). Although some may argue that creating the StringBuilder instance will slow down the method due to its memory allocation, I would like to remind everyone that string concatenation creates a lot of allocation for Garbage Collector to clean up. It is considered that ~5 string concasts are equal to creating instances of StringBuilder, so if a list consists of 5 or more elements the performance of this method will actually increase. And for larger collections this method will work several times faster.

Example of good coding practices

Let us go through another example. Consider User class (just for demonstration purposes):

public class User
{
    public bool HasConfirmedEmail { get; set; }
    public bool HasActiveAccount { get; set; }
    public bool IsLoggedIn { get; set; }
    public bool HasPremiumAccount { get; set; }
    public bool StatusAutoGenerated { get; set; }
    public DateTime LastStatusChange { get; set; }

    public bool SetStatus(string status)
    {
        // write to Data Base
        return true;
    }
}

And a method that is responsible for updating user status that has to check if all properties are in correct state:

public string UpdateStatus(string status, User user)
{
    if (user.HasActiveAccount)
    {
        if (user.HasConfirmedEmail)
        {
            if (user.IsLoggedIn)
            {
                if (user.HasPremiumAccount)
                {
                    if (!user.StatusAutoGenerated)
                    {
                        if (user.LastStatusChange < DateTime.Now.AddDays(-1))
                        {
                            if (user.SetStatus(status))
                            {
                                return "Updated";
                            }
                        }
                    }
                }
            }
        }
    }
    return "Fail";
}

Although this code is not as unclear as the first one presented in this article, it still does not follow the best clean code practices. Here is an example of how it can be done a little bit better:

const string OK = "Updated";
const string FAIL = "Fail";

public string UpdateStatus(string status, User user)
{
    if (!CanUpdateStatus(user)) return FAIL;
    if (!user.SetStatus(status)) return FAIL;
    return OK;
}

public static bool CanUpdateStatus(User user)
{
    if (!user.IsLoggedIn) return false;
    if (!user.HasActiveAccount) return false;
    if (!user.HasConfirmedEmail) return false;           
    if (!user.HasPremiumAccount) return false;
    if (user.StatusAutoGenerated) return false;
    if (!(user.LastStatusChange < DateTime.Now.AddDays(-1))) return false;
    return true;
}

The main modifications that have been made to this code to increase its readability and maintainability:

1. A static method has been created to check if the user status can be updated. This makes the UpdateStatus method way simpler to understand at first glance. Also, the CanUpdateStatus method logic can now be reused in other parts of the system as it is public and static.
2. All the “if” statements have been reversed to reduce nesting. Now almost all of parentheses are gone and it is much easier to read or debug through the code. Another advantage of this convention is its scalability. Imagine that the User class has now three more properties that need to be checked before updating status – 3 more “if” statement nestings would be necessary. Now you can only add three more lines in the CanUpdatedStatus method.
3. Strings that represent messages were removed from bodies of the methods and were put in constant variables. This also helps with maintaining the code because regardless of the number of usages of the code there is only one place where you have to change the content of the message if needed. To be honest though, all the texts should be transferred to an external resources project.

Useful links

But how can you know what conventions you should use when writing a code? Where to look for all good practices? And where to find all (or most) of this knowledge?

For C# coding conventions I recommend:

http://msdn.microsoft.com/en-us/library/ff926074.aspx

Although these examples are written in C# all the same principles apply to Java. For Java coding conventions I recommend:

https://google-styleguide.googlecode.com/svn/trunk/javaguide.html

And if you are interested in writing code that is not only useful but also readable, check out this website:

http://code.tutsplus.com/tutorials/top-15-best-practices-for-writing-super-readable-code–net-8118

Conclusion

I have participated in the GET .NET conference recently. A number of speakers talked about countless benefits of writing clean and understandable code. An argument that convinced me most was a quote from Udi Dahan who said: “Write your code as if the next programmer that will work with it is psychopath serial killer that knows where you live”. Now go and refactor your code and let me know how it changed your coding experience.

Tags: .NET, Best practices, C#, Software