New Technologies, Old Practices

I was chatting with an ex-colleague a few weeks ago and we were discussing new technologies, and how in today’s world of software development we’re redefining fundamental questions like “how we store data” or “where we execute code” that we haven’t done for a while. Javascript is becoming more and more powerful, with quicker browser … Continue reading New Technologies, Old Practices


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… // Create a new instance of the data tier Client.Project.Server.Data.DataAccessLayer empDal = new Client.Project.Server.Data.DataAccessLayer(); // Go through all employees in the employee collection. foreach (var e in empDal.GetActiveEmployees ()) { // If the employee is older than 50, send a retirement information pack. if (e.Age > 50) { e.Mailshots.Add (empDal.LoadMailer (Client.Project.Server.Enums.Mailers.RetirementPack)); } // If the employee has been working for less than 30 days and works in either the Developer, Project Manager, or Sales teams, send a welcome pack. if (DateTime.Today.Subtract (e.StartDate).TotalDays 50) { TheEmployee.Mailshots.Add (DataTier.LoadMailer (Client.Project.Server.Enums.Mailers.RetirementPack)); } // If the employee has been working for less than 30 days and works in either the Developer, Project Manager, or Sales teams, send a welcome pack. if (DateTime.Today.Subtract (TheEmployee.StartDate).TotalDays RETIREMENT_AGE) { TheEmployee.Mailshots.Add (DataTier.LoadMailer (Client.Project.Server.Enums.Mailers.RetirementPack)); } // If the employee is a new joiner and works in either the Developer, Project Manager, or Sales teams, send a welcome pack. if (DateTime.Today.Subtract (TheEmployee.StartDate).TotalDays RETIREMENT_AGE) { TheEmployee.Mailshots.Add (DataTier.LoadMailer (Mailers.RetirementPack)); } // If the employee is a new joiner and works in either the Developer, Project Manager, or Sales teams, send a welcome pack. if (DateTime.Today.Subtract (TheEmployee.StartDate).TotalDays RETIREMENT_AGE; if (EmployeeIsOfRetirementAge) { TheEmployee.Mailshots.Add (DataTier.LoadMailer (Mailers.RetirementPack)); } bool EmployeeIsANewJoiner = (DateTime.Today.Subtract (TheEmployee.StartDate).TotalDays RETIREMENT_AGE; if (EmployeeIsOfRetirementAge) { SendMailer (TheEmployee, Mailers.RetirementPack); } bool EmployeeIsANewJoiner = (DateTime.Today.Subtract (TheEmployee.StartDate).TotalDays < NEW_JOINER_DAYPERIOD); bool EmployeeIsInASoftwareDepartment = new string [] { "Developer", "Project Manager", "Sales" }.Contains (TheEmployee.Department); if ( EmployeeIsANewJoiner && EmployeeIsInASoftwareDepartment) { SendMailer (TheEmployee, Mailers.WelcomePack); } } 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. private static void SendMailer (this Employee TheEmployee, Mailers Mailer) { TheEmployee.Mailshots.Add (DataTier.LoadMailer (Mailer)); } 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 , Dictionary etc. etc.). Armed with this technique, we can now rewrite our main code as follows: foreach (var TheEmployee in DataTier.GetActiveEmployees ()) { if (TheEmployee.IsEligibleForRetirementPack()) { TheEmployee.SendMailer (Mailers.RetirementPack); } if (TheEmployee.IsEligibleForNewJoinerPack()) { TheEmployee.SendMailer (Mailers.WelcomePack); } } 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. var Employee = new Employee (); Assert.IsNull (Employee.Department); Assert.AreEqual (0, Employee.Age); Assert.IsNull (Employee.Name); Assert.AreEqual (DateTime.MinValue, Employee.StartDate); 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.