Got more questions? Find advice on: SQL | XML | Regular Expressions | Windows
in Search
Welcome to AspAdvice Sign in | Join | Help

C# Nuggets

Code Smells in C#

A lot of my development is done with Code Smells. Whether I'm working on my own code or refactoring someone elses code, if I see some code and it smells funny, then I investigate further.

Recently I've come to realize there's a code smell which we can blame Microsoft (Anders) for! 

C# Code Regions. 

Back in the early days of .Net (ie 2000-2003), I used to moderately region my code. But since reading Martin Fowler's seminal Refactoring book where the concept of Code Smells was introduced, I found that I didn't need C# Regions.

So why are regions a Code Smell?

There are two types of regioning I've seen in code. Regions that group methods, properties, member variables, constructors etc together, and those which collapse a block of code. I'll deal with each separately.

Using Regions to group members.

In many companies, its commonplace (and even enforced practice) to group similar class members together and place a region block around them. I've even seen this as a coding doctrine inside Microsoft. The problem is that Visual Studio already has a facility to do this automatically. And it's been around in VS since it was called VJ++ and before that in VC. It's called the Class View window, accessed with the shift-ctrl-C shortcut. So why anyone would care about building a class "DOM" into their code using Regions, rather than rely on the Class View window I have no idea. Perhaps they're using Notepad?

Using Regions to hide blocks within a method.

This is when it gets really nasty. I've seen multi-hundred line long methods containing regions, and subregions and sub-subregions. I've seen switch() statements wrapped in a region, then each individual case have a region, then subregions inside of the region inside of the case. Switch statements are already a Code Smell, but having region after region after region is a sure-fire way of telling you the method is too long.

 
So, please, can we all resolve to stop using Regions?

Interestingly, there are many features that Java has started copying from C# - but regions were not one of those. Perhaps in C# v4 Microsoft can right the wrong and deprecate regions?

Sponsor
Published Monday, May 12, 2008 5:11 PM by rbirkby
Filed under: , ,

Comment Notification

If you would like to receive an email when updates are made to this post, please register here

Subscribe to this post's comments using RSS

Comments

 

Salvatore Aiello said:

I agree! All to often I come across code filled with regions and I have to spend about 10 minutes before I will allow myself to do any work just deleting them. I just dont understand why people dont generally use class view (or the sourcecode outliner pluggin http://www.visualstudiogallery.com/ExtensionDetails.aspx?ExtensionID=bea9ed59-8857-4032-9666-9af1c1a33969)
June 8, 2008 8:24 PM
 

Alex said:

So, it's been some time since I've written any C# code, but my favorite use of regions to provide a quick overview and enhance the readability of the code. Basically, collapsed, the regions would read as pseudo-code, with the ability to expand each region to get the actual code. But that's is also my style of coding... Often times I'll write all the inline comments of a method as my pseudo-code, and then fill in with functional code. Using regions excessively or in a nested format would definitely be an issue in readability, best practices, and redundancy. But I believe that there are appropriate use cases for regions, and being able to read in-line rather than in class view can be much more convenient. Thanks for the post. -- Alex
July 7, 2008 1:43 PM
 

rbirkby said:

Alex: No - code _structure_ enhances code readability. Not comments, regions or other ephemeral AST nodes.

Here's how I write code; like you, I'll write inline comments of a method as pseudo-code, then fill in the actual code later. But crucially, I'll then refactor that method into a number of other, smaller methods. In the end, I find that the block of code headed by each inline comment has become a method in its own right - and I find that the inline comment has simply become the name of each method. The end result is that I can remove all those inline comments because they just repeat the name of my new method.

By having small methods, you will find a massive reduction in your need for temporary variables. It will be easier to reason about the behavior of a method when you come back to it in the future. The .Net JIT heuristics will have a better chance to optimize your method (CLR JIT inlining is only performed for small methods), giving better performance. You will reduce code redundancy and increase code re-use through a better understanding of what the behavior of each method is, and how it can be used by other code. Finally, the noun-verb (ie class-method) structure of your code becomes the atomic readability scope for your program.

That's why regions are a code smell.

July 7, 2008 2:34 PM
 

foobar said:

I think I just threw up in my mouth a little. For some reason, I suspect that coding like the way to you won't lead me to the promised land of more maintainable code and fewer bugs. captcha = absurd How ironical.
July 12, 2008 12:01 PM
 

rbirkby said:

foobar: Have you read Fowler's Refactoring?

I've spent a lot of time over the past 4 years refactoring other people's C# codebases. I'm always amazed at the number of bugs I find by refactoring their code in the way I describe. For me, that's all the proof I need that coding as Fowler advocates works.

July 12, 2008 12:19 PM

Leave a Comment

(required) 
(optional)
(required) 
Enter the code you see below

Submit