C# regions, inline comments and blank lines are not harmless

I didn’t want to call this “C# regions considered harmful” because that article has already been written – and even though there’s a backlash against articles with titles like this (¨Considered harmful¨ essays considered harmful), Rob Meyer’s article is spot on and should be read by every C# developer that uses regions in their code.

I don’t understand why regions were introduced into C#.  I guess they were meant to allow tools to hide generated code before partial classes were introduced.  Now they're just in the way.  I think I dislike them so much because they appear to be benign – you would think that a keyword that does nothing couldn’t do any harm.  But hiding code, without abstracting it away using proper OO techniques, encourages developers to write poorly structured code.

The default settings in Visual Studio that cause regions to be collapsed when you open a file don’t help (although you can change that: Tools –> Options –> Text Editor –> C# –> Advanced –> Enter outlining mode when files open).  If a file has collapsed regions in it I can press Ctrl-ML to expand them all in one go. Even better, I can take the regions out.  ReSharper has a File Structure tool window (Ctrl-F11) where you can delete a region by clicking the “x” in its top right corner.  To stop ReSharper adding regions when you format code: ReSharper –> Options –> C# –> Type Members Layout –> uncheck Use Default Patterns, then remove all <Group /> tags from the XML.

It’s not only regions that are used to delineate chunks of code.  Often inline comments, and even blank lines, are used in a similar way.  The comment’s text often describes what the chunk of code does.  Horrible.  Extract a method.  Or move the responsibility to another (maybe new) class.  Every time I find myself adding a blank line or an inline comment, I ask myself why?  Am I dividing up code?  Should I be calling a (new) method?  I'm not saying that inline comments are necessarily bad - they should just be reserved for stating the non-obvious.  Nor are blank lines necessarily bad - they should just be used to improve readability, not to divide a monolith.

A method should do one thing, and one thing only (Jeff’s post on this is great).  I should be able to grok what a method does instantly.  The first (major) clue to a method’s functionality is it’s name (which is really a Pascal cased sentence with a verb and everything!).  The name (and signature) tells me what I should expect the method to do. Then the body of the method confirms that.  A few lines of code that I can see in one glance and understand quickly.  A few lines of code that do one thing.  TDD/BDD helps keep our methods short and to the point.

A class should do one thing, and one thing only. The Single Responsibility Pattern (SRP) should keep our classes focused and cohesive.  No room for regions.  No need for regions.  If we adhere to the SRP then our classes should be fairly short – with not too many members.  That means that wrapping members in regions (with names like “.ctors”, “fields”, “properties”, “methods” etc.) is totally unnecessary, obstructive and thus counterproductive (although ordering the members within the class is essential).  The goal here has got to be: “I want other developers to be able to read and understand my code as quickly and as easily as possible” – meaning that I don’t want them to have to lift up all these region "carpets" to see what’s been swept underneath.