Writing Better Code – Part 1


I started writing this post about commenting in general, but then I ended up thinking about writing cleaner, simpler code in general. I guess what I’m writing about here are ways to say more with less.

Everyone gets taught “comment your code” as the first step to documenting your code. But there’s a few things that I’ve been reading and thinking about when writing code in order to effectively make your code comment itself i.e. self-documenting code.

The thing that I don’t like about comments is that they are a cop-out for you as the developer to let you get away with writing hard-to-read / understand / debug code – all you need to do is put a comment on the top which states what the intent of your method or bit of logic is, and you don’t need to feel guilty for not commenting. Great, some other developer will see what you are trying to do – but when it comes to debugging it in a year’s time to fix that “critical” bug the client is claiming means they cannot get any work done, (at all, ever), will the developer understand how your 10-level-deep if/then routine works? Or why you wrote the code “that way”?

I used to think that virtually all code should be commented, aside from absolutely obvious examples. However, I now tend to think that whilst, yes, there is a need for commenting, there are several other ways of documenting your code in VS that can tell you a lot more about what your intent in a piece of code is, or how to consume a class, than comments.

I’ve created a contrived scenario below which illustrates a sort of “before and after” approach to replacing use of comments with other options which are useful to have in your arsenal. I’ve taken most of these techniques from the mighty Code Complete 2 book, but some of the newer ones, like Extension Methods, have only been around a year or so.

The Example Mailshot Application

Here’s an example of some code for a fictional routine which runs the internal employee mailshot and how we might write it. Then I’ll show a few ways to (allegedly) make it easier to read…

image

OK. I bet that when you were reading the code above, you read the comments, scanned once through the code, had a quick look at some words in the code that matched the comments, and thought “ok, makes sense”. That’s what I do mostly when I’m reading my (or someone else’s) code. In reality though, if you take away those comments, I don’t think that the code is that readable. The problem is that we are reliant on the comments to explain both the what, and the how, of the code. Unfortunately, when it comes to maintaining this code, comments are often useless in explaining why you’ve done what you’ve done – or else there are so many of them that they get in the way.

There are a few things that we can do to make this code both “simpler” on the eye, whilst also more maintainable and / or less complex.

1. Use “intelligent” variable and method naming

First thing is to make the variable names self-describing, and then remove the unnecessary comments:

image
It’s now obvious that what was “e” (now employee) is actually an employee and that empDal is actually the data tier.
 

2. Make good use of constants

Not only does using constants make code more fault resilient i.e. a single place to store a business logic value, but it also gives you a chance to explain the meaning of the value with the const’s name. Using this below means that already some parts of the comments are becoming redundant.

image

 

3. Remove unnecessary variable types and namespaces

This drives me up the wall sometimes. Why do we still fully resolve all our types and classes? It’s almost pointless in my mind – you have intellisense, so you can easily see the namespace, or better yet just hit F12 to go straight to definition of the type, have a look around, and then hit “CTRL –“ to go back to where you were

If you don’t have the appropriate Using statement in your code, hitting “CTRL .” will automatically generate the appropriate using statement for you – smart 🙂

Also – use the var keyword where it’s obvious what the type is (either from the right hand side of the assignment, or from the variable name itself). This again cuts down on “junk” in your code, making it more readable, but more importantly, saves you vital seconds of when coding, leaving you more time in the day to surf Facebook / watch 24 etc. etc.

image

 

4. Avoid complex conditional statements

This is something that has helped me no end when explaining to myself what I’m doing later on when reviewing my code. For example, I found some old code when rewriting an old application where I had an IF statement with about 5 complex conditions in there. Above the IF, I had a comment section maybe four or five lines long explaining what I was doing. Using the following technique I was able to remove the comment completely whilst still improving the readability of the code.

The idea of this is that you split complex IF statements into smaller chunks, thus making the overall IF statement easier to read.

image

I’ve now gotten to a position where I’ve also removed the comments above both IF statements as they are just repeating the conditionals below them, except for one last bit (see the next tip). Notice that I’ve also replaced the somewhat ugly “1 || 2 || 3”-style syntax with a simple Contains() statement to test if the employee is in the right department.
 

5. Separate disparate pieces of logic into individual methods

The “last bit” I was referring to above is the actual sending of a mailshot: we still have the Mailshots.Add line. This is an ugly line of code – you can’t look at this line “at a glance” and know that you’re sending a mailshot – you have to read the single line of code and split it into chunks step-by-step to see what it’s doing:

  1. You’re adding some sort of Item to the Mailshots collection (a logical name at least)
  2. “Item” is the result of the LoadMailer () call
  3. LoadMailer probably returns a mailshot item object on the enum you’re passing in – in this case, RetirementPack.
  4. You then probably read the above steps backwards (right-to-left) and think “ok, makes sense – we’re loading a Retirement mailer and adding it to the outbox of the employee”.

This is much more readable though:

image

All I’ve done is refactor the code to separate out the “nitty gritty” of what is going on under the bonnet and left the intent i.e. “send a mailer of type X” in this method.
 

6. Use Extension Methods to simplify intent

Extension methods are a great way to make code look more logical from a consumer perspective without polluting the serving objects. In the code above, the SendMailer call is really acting on TheEmployee all the time – that’s the “central” object. You would like to be able to do something like TheEmployee.SendMailer(), right? However, perhaps we don’t want to add the SendMailer call to the Employee object – maybe that will break our architecture, or maybe we aren’t even in control of that Type at all e.g. it’s a third-party assembly.

This is where extension methods are a godsend. Here’s the code for the SendMailer method, as an Extension Method. The only thing you need to do with an Extension method is to put the “this” keyword before the type that you want to extend (in our case, Employee), and ensure that the method is static, in a static class.

image

In reality, the compiler re-writes your code as per the original version of the method (where you pass in the Employee object) – but as far as you are concerned, as the consumer of the Employee class, you can now call the SendMailer method on an Employee as if it was part of it. You can do this with ANY type, whether you own it or not. Bear in mind though that this does not give you access to private methods or anything like – it’s just a syntactic “trick” to look at though that method lives on the class. (Side note – this is where 99% of all the LINQ methods come from – System.Linq.Enumerable contains a load of extension methods that live on top of anything that implements IEnumerable. This is how Microsoft were able to release .NET 3.5 on top of .NET 2 without rewriting all the existing IEnumerable classes e.g. List <T>, Dictionary <T> etc. etc.).

Armed with this technique, we can now rewrite our main code as follows:

image

I’ve also gone and pulled out the business logic to calculate whether an employee is suitable for a Retirement and NewJoiner pack and placed them into extension methods as well. This code above has no comments whatsoever, yet is much, much easier to read at a glance than the original code.
 

A good test for this would be to place it into VS2008 Team System and run the code analysis on it – what scores would it get for Code Complexity etc. before and after?

7. Use XML tags everywhere 🙂

I’m not going to demo XML tags as we all know what they are – suffice it to say that I believe that all methods and properties that are either public, internal or protected should have XML tags. Use GhostDoc to save you time – CTRL+SHIFT+D will generate you a comment for your method, and maybe 60%-70% of the time it gets the comment spot on.

If you find that you need to start commenting private methods, it’s usually a sign that your class is doing too much on its own and should be split into separate classes (this is the “Single Responsibility Priniciple” in action).

8. Use Unit Tests as a form of documentation by example

Units Tests are a great way – overlooked in my opinion – of documenting for others how to consume your classes. The reason that they are a great way is because they demonstrate, by example, how to interact with your classes, and what results they give under what circumstances. You don’t have to write long documents or wikis etc. explaining how to interact with a class – or have to maintain it afterwards – because your unit tests are always the most up-to-date way of documenting your API.

I always, for example, have a basic unit test for every class of mine that says: create a new instance of my type. Then assert (test) that every property is as I expect e.g.

image

Someone can now look at this code and know what state a new Employee object will be if they were to create one.
 

9. Use Class Diagrams instead of static Word Documents etc.

Class diagrams are a good way of showing interacts with complex classes at a glance without having to write reams of text – plus because they live within VS, they are instantly accessible by the developers, plus they (generally) auto-update themselves quite well.

There are a few other examples floating about that I took from Code Complete that I’ll blog about in the next weeks – it’s a great book to read, it really made me think about how I code, but it’s pretty heavy going and in some places pretty dry. And it’s 99% not C# – the majority of the examples are in either VB or C.

Advertisements

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s