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
 

Tony said:

I agree adding regions in within a method is bad.  I disagree with using regions in a class is bad.  I like seperating clear sections for readability sake.  

August 1, 2008 9:40 AM
 

rbirkby said:

Tony: have you seen the class browser?

August 1, 2008 11:13 AM
 

DevTopics said:

I don't quite follow the logic here. You're suggesting we remove an organizational tool because it enables developers to create classes that you consider "too large"? Wouldn't that be like suggesting we abolish folders from the file system because it enables people to create too many files? I think code outlining (regions) are one of the greatest inventions for pure coding since the IDE. Code outlining enables a developer to hide complexity and reduce a 20-page code file down to a single screen. For a disabled developer such as myself, it's a life saver, eliminating the painful need to scroll and scroll and scroll. As for the belief that any code file which is helped by regions must be too long... Since when is there a byte limit on source code? In the real world, sometimes classes can become large. Look at System.Windows.Forms.Control or any UI Form that can have tens or hundreds of event handlers. I suppose the alternative is to use partial classes and spread a class among several files, but then you've simply swapped one organizational tool (regions) for another (files and folders). For anyone who has built non-trivial, real-world business applications, code outlining is a terrific organizational tool.
August 7, 2008 1:56 PM
 

Rainer Hilmers Developer-Blog said:

Thomas Mentzel stellt diese Frage heute in seiner Reihe “ Fragen der Woche #2 ”. Ich kann mich erinnern

April 7, 2011 5:21 PM
 

Rainer Hilmers Developer-Blog said:

Thomas Mentzel stellt diese Frage heute in seiner Reihe “ Fragen der Woche #2 ”. Ich kann mich erinnern

April 7, 2011 5:22 PM
 

akjsohi said:

Sorry but I don't agree with this reasoning. MS don't you dare to remove Regions.
September 16, 2011 8:45 AM
 

rbirkby said:

@akjsohi Care to explain why?

September 16, 2011 8:50 AM
 

Kevin said:

I agree 100% that long classes are an anti pattern, but i like to use regions to organize my classes into Fields, Constructors, Properties, Public methods, and Private methods. And even with these added #region and #endregion lines (10 extra lines in total), my classes are still only 50-100 lines of code long...So I wouldn't necessarily say that #region's are an antipattern, I think its just when you have lots of lines in a class its an antipattern
January 15, 2012 5:16 PM
 

rbirkby said:

@kevin why do you group fields/properties/ctors/methods?

January 16, 2012 2:24 AM
 

sowen said:

i can think of 2 good reasons of it: 1) for making the editor tiny, reduces code blocks while I am still working on it 2) when doing code generation, it's very useful to generate a long class, it's nice to group similar blocks and collapse, so after the code is generated, i can get a good overview at the first glance so, region is very useful, not a bad smell
March 25, 2012 3:56 PM
 

rbirkby said:

@sowen Are you saying your classes are so large that the built-in method collapsing isn't enough and you need to add regions to allow you to collapse portions of the class to make it a manageable length?

I think you need to read up on the single responsibility principle.

As for codegen - that's what partial classes are for.

March 25, 2012 4:44 PM
 

sowen said:

sigh...i have used many different editors and written different language in the past over 10 years, u can talk about all principle books i have read, but when u r in real business world, it's always easy to get a class file that has 1k lines of code but still in very good shape. believe it or not, all princile of having smaller classes or less code per file, oh year, they are beautiful talk. reality is different. a project can still be loose coupling and well constructed when some files have more than 1k lines of code. i want my editor looks tidy and locate easily to the section of fields/properties/ctors/methods (even group methods in a way). about the code generation, i don't think u understand what i was talking about. so, end of the story, if u haven't felt the benefit of having regions, u will feel it, eventually, give it a time, or work in some projects with more than 100 people.
March 26, 2012 8:28 AM
 

rbirkby said:

@sowen When I wrote this blog entry, I'd just finished refactoring a .cs file with 6,000 lines including a 700 line method. I've just checked back in source control history and that file had 22 #region statements in it.

So I understand the pain. But ask any doctor and they'll tell you the answer to pain is not to mask it with painkillers. That just makes the problem worse. The answer is to fix the root cause. One way to do that is to make every developer feel the pain and this introduces a negative feedback loop - something I highly recommend. #PitOfSuccess

I think it's unlikely I'll re-discover regions; I've been developing C# code since 2000. Back then, 12 years ago, I used regions liberally. Then around 2005 I discovered the error of my ways. It seems many many other C# developers have too: https://www.google.com/search?q=region+code+smell

March 26, 2012 9:10 AM
 

rbirkby said:

@sowen I also notice that Microsoft's .Net Developer Platform StyleCop rules forbids regions:

"Per ADP guidelines, the use of regions is not allowed (copied from CSD guidelines doc)"

http://aspnetwebstack.codeplex.com/SourceControl/changeset/view/c5fcf6b687f0#Settings.StyleCop

March 28, 2012 6:08 PM

Leave a Comment

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

Submit