[EN] Challenge everything

[read Polish version here]

Today, I challenge my coding style. Once again I decided to use my experience to revise my way of writing code. Here’s what I made up.

Var

Some time ago, as part of my ‘readable code’ post series, I criticized the var keyword for ist uselessness. Guess what happened since then? I changed my mind! Var is not scarring me any more. I got used to it. I was convinced by the possibility of using longer, more expressive variable names without making my code lines longer (I have a limit of about 100 characters).

Regions

Nothing changed here. Maybe I don’t like them even more then few months ago. I use regions only in exceptional cases when there is no other option (of refactoring). For example, in DDDSample.NET I use regions in ValueObject classes to hide the infrastructure code like equality operators. Once this code have been written, it is so dead simple that nobody will care about it so it could be hidden in a region.

Interface implementations

I used to wrap interface implementations in regions (as does R# by default). Recently I stopped doing this. First, I really rarely write classes that implement more than one non-infrastructure interface. Second, if I do implement an interface, it is a rare case that this class has also other, non related to that interface, public methods. There is no reason to distinguish the interface-implementing methods from all the others.

There is yet another argument against wrapping interface implementations in regions. Doing so makes it impossible to correctly lay out methods. Why?

Public methods, private methods

Unce Bob, in his excellent Clean Code convinced me that methods should be placed according to calling patterns. Caller method should be placed immediately above it’s callees. The main benefit is that one doesn’t have to scroll screen up and down to read the code. It is also a logical approach. Think about it. The relation ‘is calling’ creates far grater coupling between methods than having the same access type (private, public). Coupled methods should be placed close to each other in code, because there is big probability that readers would have to jump between them in order to understand the behaviour. Let’s look at the example. Here is how I used to write code:

public Class1()
{
   public void MethodA() {
   }
   public void MethodB() {
   }

   private void MethodA_1(){}
   private void MethodA_2(){}
   private void MethodB_1() {}
   private void MethodB_2() {}
}

And here is how I do it now:

public Class2()
{
   public void MethodA() {}
   private void MethodA_1(){}
   private void MethodA_2(){}

   public void MethodB() {}
   private void MethodB_1() {}
   private void MethodB_2() {}
}

You can see that while old style enables wrapping interfaces in region, the new one does not. I would have to wrap the whole class which is pointless.

Fields

Reading of NServiceBus code taught me that I can break the member ordering dogma (first fields, then methods). In NSB I encountered a pattern: first methods, then constructors, then fields. I was a little sceptic at first, but as time passed I got used to it and, eventually, I adopted it. I don’t know what was Udi’s original motivation, but for me the main benefit is that I open the class in my IDE and immediately I see all that is interesting — the public API. Thanks to modern IDEs chances are that I won’t need to see the fields at all when reading class code.

Constructor

Classes I write are either full-featured ones or merely data structures without any behaviour. Instances of the latter are created using good old new() operator, so constructor signature does matter. In these cases I put the constructor close to the top of the class code, probably after fields, but before properties. These pseudo-classes have classic member order.

Full-featured classes are completely different. Their instances are usually created by DI framework, and I don’t care what it needs to do it’s job. I put the constructor near the bottom, just above fields. I don’t want to see it normally, only when I need to add yet another dependency to the class.

End result

Here’s how it looks:

public MyNewClass()
{
   public void DoSomething()
   {
      HelperMethod1();
      HelperMethod2();
   }

   private void HelperMethod1()
   {
   }

   private void HelperMethod2()
   {
   }

   public void DoSomethingElse()
   {
   }

   public void MyNewClass(SomeOtherClass dependency, YetAnotherClass anotherDependency)
   {
      _dependency = dependency;
      _anotherDependency = anotherDependency;
   }

   private readonly SomeOtherClass _dependency;
   private readonly YetAnotherClass _anotherDependency;
}

What do you think about it? If anyone liked that coding style, here you can download an XML formatting specification for your ReSharper. It implements what I’ve written about plus if you have a field called ‘_logger’ it will placed on the very bottom of you class.