Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

In C# code it allows the #region/#endregion keywords to made areas of code collapsible in the editor. Whenever I am doing this though I find it is to hide large chunks of code that could probably be refactored into other classes or methods. For example I have seen methods that contain 500 lines of code with 3 or 4 regions just to make it manageable.

So is judicious use of regions a sign of trouble? It seems to be to me.

share|improve this question
4  
FYI: CodeMap pretty much eliminates the needs for regions. visualstudiogallery.msdn.microsoft.com/… - Click and it takes you to the method/attribute/whatever. I use it every day and it's amazing how much you gain in productivity and in cognitive terms as well. You get a much clearer 'birds eye view' of the class. –  Sergio Feb 28 '11 at 23:11
4  
Can anyone make this a wiki? There is no right or wrong answer to this question (well, within reason), it's almost completely subjective. –  Ed S. Feb 28 '11 at 23:21
1  
For what it's worth, Jeff Atwood hates them. He argues that they hide bad code. –  Brian Jun 5 '12 at 18:11
show 3 more comments

16 Answers

up vote 42 down vote accepted

A code smell is a symptom which indicates that there is a problem in the design which will potentially increase the number of bugs: this is not the case for regions, but regions can contribute creating code smells, like long methods.

Since:

An anti-pattern (or antipattern) is a pattern used in social or business operations or software engineering that may be commonly used but is ineffective and/or counterproductive in practice

regions are anti-patterns. They require more work which doesn't increase the quality or the readability of the code, which doesn't reduce the number of bugs, and which may only make the code more complicate to refactor.

Don't use regions inside methods; refactor instead

Methods must be short. If there are only ten lines in a method, you probably wouldn't use regions to hide five of them when working on other five.

Also, each method must do one and one only thing. Regions, on the other hand, are intended to separate different things. If your method does A, then B, it's logical to create two regions, but this is a wrong approach; instead, you should refactor the method into two separate methods.

Using regions in this case can also make the refactoring more difficult. Imagine you have:

private void DoSomething()
{
    var data = LoadData();
    #region Work with database
    var verification = VerifySomething();
    if (!verification)
    {
        throw new DataCorruptedException();
    }

    Do(data);
    DoSomethingElse(data);
    #endregion

    #region Audit
    var auditEngine = InitializeAuditEngine();
    auditEngine.Submit(data);
    #endregion
}

Collapsing the first region to concentrate on the second is not only risky: we can easily forget about the exception stopping the flow (there could be a guard clause with a return instead, which is even more difficult to spot), but also would have a problem if the code should be refactored this way:

private void DoSomething()
{
    var data = LoadData();
    #region Work with database
    var verification = VerifySomething();
    var info = DoSomethingElse(data);

    if (verification)
    {
        Do(data);
    }

    #endregion

    #region Audit
    var auditEngine = InitializeAuditEngine(info);
    auditEngine.Submit(
        verification ? new AcceptedDataAudit(data) : new CorruptedDataAudit(data));
    #endregion
}

Now, regions make no sense, and you can't possibly read and understand the code in the second region without looking at the code in the first one.

Another case I sometimes see is this one:

public void DoSomething(string a, int b)
{
    #region Validation of arguments
    if (a == null)
    {
        throw new ArgumentNullException("a");
    }

    if (b <= 0)
    {
        throw new ArgumentOutOfScopeException("b", ...);
    }
    #endregion

    #region Do real work
    ...
    #endregion
}

It's tempting to use regions when arguments validation starts to span tens of LOC, but there is a better way to solve this problem: the one used by .NET Framework source code:

public void DoSomething(string a, int b)
{
    if (a == null)
    {
        throw new ArgumentNullException("a");
    }

    if (b <= 0)
    {
        throw new ArgumentOutOfScopeException("b", ...);
    }

    InternalDoSomething(a, b);
}

private void InternalDoSomething(string a, int b)
{
    ...
}

Don't use regions outside methods to group

  • Some people use them to group together fields, properties, etc. This approach is wrong: if your code is StyleCop-compliant, then fields, properties, private methods, constructors, etc. are already grouped together and easy to find. If it's not, than it's time to start thinking about applying rules which ensure uniformity across your codebase.

  • Other people use regions to hide lots of similar entities. For example, when you have a class with hundred fields (which makes at least 500 lines of code if you count the comments and the whitespace), you may be tempted to put those fields inside a region, collapse it, and forget about them. Again, you are doing it wrong: with so many fields in a class, you should think better about using inheritance or slice the object into several objects.

  • Finally, some people are tempted to use regions to group together related things: an event with its delegate, or a method related to IO with other methods related to IO, etc. In the first case, it becomes a mess which is difficult to maintain, read and understand. In the second case, the better design would probably be to create several classes.

Is there a good use for regions?

No. There was a legacy use: generated code. Still, code generation tools just have to use partial classes instead. If C# has regions support, it's mostly because this legacy use, and because now that too many people used regions in their code, it would be impossible to remove them without breaking existent codebases.

Think about it as about goto. The fact that the language or the IDE supports a feature doesn't mean that it should be used daily. StyleCop SA1124 rule is clear: you should not use regions. Never.

Examples

I'm currently doing a code review of my coworker's code. The codebase contains a lot of regions, and is actually a perfect example of both how to not use regions and why regions lead to bad code. Here are some examples:

4 000 LOC monster:

I've recently read somewhere on Programmers.SE that when a file contains too many usings (after executing "Remove Unused Usings" command), it's a good sign that the class inside this file is doing too much. The same applies to the size of the file itself.

While reviewing the code, I came across a 4 000 LOC file. It appeared that the author of this code simply copy-pasted the same 15-lines method hundreds of times, slightly changing the names of the variables and the called method. A simple regex allowed to trim the file from 4 000 LOC to 500 LOC, just by adding a few generics; I'm pretty sure that with a more clever refactoring, this class may be reduced to a few dozens of lines.

By using regions, the author encouraged himself to ignore the fact that the code is impossible to maintain and poorly written, and to heavily duplicate the code instead of refactor it.

Region “Do A”, Region “Do B”:

Another excellent example was a monster initialization method which simply did task 1, then task 2, then task 3, etc. There were five or six tasks which were totally independent, each one initializing something in a container class. All those tasks were grouped into one method, and grouped into regions.

This had one advantage:

  • The method was pretty clear to understand by looking at the region names. This being said, the same method once refactored would be as clear as the original.

The issues, on the other hand, were multiple:

  • It wasn't obvious if there were dependencies between the regions. Hopefully, there were no reuse of variables; otherwise, the maintenance could be a nightmare even more.

  • The method was nearly impossible to test. How would you easily know if the method which does twenty things at a time does them correctly?

Fields region, properties region, constructor region:

The reviewed code also contained a lot of regions grouping all the fields together, all the properties together, etc. This had an obvious problem: source code growth.

When you open a file and see a huge list of fields, you are more inclined to refactor the class first, then work with code. With regions, you take an habit of collapsing stuff and forgetting about it.

Another problem is that if you do it everywhere, you'll find yourself creating one-block regions, which doesn't make any sense. This was actually the case in the code I reviewed, where there were lots of #region Constructor containing one constructor.

Finally, fields, properties, constructors etc. should already be in order. If they are and they match the conventions (constants starting with a capital letter, etc.), it's already clear where on type of elements stops and other begins, so you don't need to explicitly create regions for that.

share|improve this answer
1  
I think there are at least a few defensible uses of #regions - e.g. collapsing of guard clauses (checking input parameters) which, if left uncollapsed, detract from the gist or "meat" of the method. Another example is exception handling on API boundaries; you often don't want to impact the stack trace by calling other methods to wrap the exception, so you have several lines of code to wrap and rethrow. This is also often non-relevant code, and can safely be collapsed. –  Daniel B Jun 6 '12 at 8:24
2  
Reality Check. I've seen plenty of inter-method #regions that were helpful. When you've got a several hundred line method with nested control logic with dozens to hundreds of lines w/in some of those - thank goodness the idiot at least put in regions. –  radarbob Mar 27 '13 at 22:14
2  
@radarbob: in short, regions are helpful in crappy code which shouldn't exist in the first place. –  MainMa Mar 28 '13 at 8:35
2  
-1 (if I had the reputation). Even if your type members are well organised and separated, there might be a lot of them. Regions save you having to scroll past 10 or 12 properties and the associated getters and setters just to get to a method you want to work on. The only alternative is to collapse all your properties individually, which is not something I'm a fan of. The large scale show/hide functionality regions provide is extremely useful. I agree about intra-method regions though. –  Asad Aug 1 '13 at 20:26
show 11 more comments

First off, I can't stand the term "code smell" anymore. It is used too often and is much of the time thrown about by people who couldn't recognize good code if it bit them. Anyways...

I personally don't like using a lot of regions. It makes it harder to get at the code, and the code is what I am interested in. I like regions when I have a large chunk of code that doesn't need to be touched very often. Aside from that they just seem to get in my way, and regions like "Private Methods", "Public Methods", etc. just drive me crazy. They're akin to comments of the variety i++ //increment i.

I would also add that the use of regions can't really be an "anti-pattern" as that term is commonly used to describe program logic/design patterns, not the layout of a test editor. This is subjective; use what works for you. You are never going to end up with an unmaintainable program due to your overuse of regions, which is what anti-patterns are all about. :)

share|improve this answer
1  
Normally I'd downvote you for the code smell comment but in this case it's exactly accurate. Non-code can't be a code smell. +2! –  Мסž Mar 1 '11 at 0:06
4  
Haha, well, I don't mean to say that the term "code smell" can't be used accurately. It can, but I see it thrown around so much these days that my immediate reaction is just annoyance, especially when it comes from people who are really just repeating what they hear without understanding it or thinking critically. Statements like "more than 5 local variables in a function is a code smell" just show how little experience that person actually has. –  Ed S. Mar 1 '11 at 0:11
8  
Not sure I understand your comment about code smells. Code smells don't indicate there is a problem, just there may be a problem. –  Craig Mar 1 '11 at 0:51
3  
+1 for standing up to the term code smell. I got fed up when I saw a post claiming private methods were a code smell. But overall I disagree with your distaste for regions. I'm easily distracted. In fact I'd love it if VS had a VB4 mode where you could display just a single method at a time. –  Josh Mar 1 '11 at 6:06
5  
Just wanted to chime in and say that misuse of a perfectly good metaphor shouldn't devalue the metaphor. "Code smell" is a great metaphor, one that is instantly understood, easily remembered, and easily used. There are still plenty of places where applying the "code smell" metaphor is still the best way to get a point across. –  Eric King Jun 5 '12 at 20:22
show 5 more comments

It's incredible to me how many people hate regions so passionately!

I agree completely with so many of their objections: Shoving code into a #region to hide it from view is a bad thing. Dividing a class into #regions when it should be refactored into separate classes is clearly a mistake. Using a #region to embed redundant semantic information is, well, redundant.

But none of those things mean that there's anything inherently wrong with using regions in your code! I can only assume that most people's objections come from having worked on teams where others are inclined to use IDE features like this one incorrectly. I have the luxury of working primary by myself, and I appreciate the way that regions have helped to organize my workflow. Maybe it's my obsessive compulsive disorder, but I don't like to see a bunch of code on my screen at a time, no matter how neatly and elegantly written it may be. Separating things out into logical regions allows me to collapse the code that I don't care about to work on the code that I do care about. I'm not ignoring badly-written code, it doesn't make sense to refactor it any more than it is, and the additional "meta" organization is descriptive, rather than pointless.

Now that I've spent more time working in C++, programming more directly to the Windows API, I find myself wishing that the support for regions was as good as it is for C#. You could argue that using an alternative GUI library would make my code simpler or clearer, thus eliminating the need to get irrelevant code noise off of the screen, but I have other reasons for not wanting to do that. I'm proficient enough with my keyboard and IDE that expanding/collapsing the code subdivided into regions takes less than a fraction of a second. The time I save in brainpower, trying to limit my conscious focus to only the code that I'm currently working on, is more than worth it. It all belongs in a single class/file, but it doesn't all belong on my screen at the same time.

The point is that using #regions to separate and logically divide your code is not a bad thing to be avoided at all costs. As Ed points out, it's not a "code smell". If your code smells, you can be sure that it's not coming from the regions, but instead from whatever code you've tried to bury in those regions. If a feature helps you to be more organized, or write better code, then I say use it. If it becomes a hindrance, or you find yourself using it incorrectly, then stop using it. If worst comes to worst, and you're forced to work on a team with people who use it, then memorize the keyboard shortcut to turn off code outlining: Ctrl+M, Ctrl+P. And stop complaining. Sometimes I get the feeling this is another way that those who want to be seen as "true", "hardcore" programmers like to try and prove themselves. You're no better off avoiding regions than you are avoiding syntax coloring. It doesn't make you a more macho developer.

All of that being said, regions within a method are just sheer nonsense. Any time you find yourself wanting to do that, you should be refactoring into a separate method. No excuses.

share|improve this answer
2  
Well said. Use of regions for organization is no more harmful than toggling the IDE's "view whitespace" option. It's a personal preference. –  Josh Mar 1 '11 at 6:50
4  
Working on WPF with ViewModels that have 10 or 20 properties that simply wrap properties on my model, I love regions - I can just tuck those properties away in a region (they never need to be touched) and keep my eyes on the relevant code. –  Kirk Broadhurst Mar 1 '11 at 6:57
1  
Totally agree. In .NET 2.0, properties are about 8-10 lines long. When you have over 20 properties in a class, they take up a lot of space. Regions are perfect for collapsing them. –  Kristof Claes Mar 1 '11 at 7:50
1  
I'm willing to wager my left ball that people who hates regions have either never developed a WPF application, or never really used WPF's defining functionalities such as data binding and command binding. Just setting up your code up to make those work takes up a lot of space and you generally don't have to look at them again. –  l46kok Nov 28 '12 at 7:53
show 1 more comment

Yes regions are a code smell!

I'd be happy to see regions removed from the compiler altogether. Every developer comes up with his or her own pointless grooming scheme that will never ever be of value to another programmer. I has everything to do with programmers wanting to decorate and beautify their baby, nothing to do with any real value.

Can you think of one example where you though "geez, I wish my collegue has used some regions here!"?

Even though I can configure my IDE to automatically expand all regions they are still an eye-soar and detract from reading the real code.

I'd really care less if all my publics are bunched up together or not. Congratulations you know the difference between a variable declaration and initialization, no need to display it in code!

Valueless grooming!

share|improve this answer
1  
The compiler would need to ignore them when parsing your C# code. If it doesn't ignore them it would throw up on them. I mean it that way. –  Joppe Dec 13 '12 at 1:55
show 1 more comment

Using #region blocks to make a very large class readable is typically a sign of violating the Single Responsibility Principle. If they're being used to group behavior, then it's also likely that the class is doing too much (once again violating SRP).

Sticking with the "code smell" line of thought, #region blocks aren't code smells in and of themselves but instead they're more "Febreze for code" in that they try to hide smells. While I've used them a ton in the past, as you start refactoring you start seeing fewer because they end up not hiding much.

share|improve this answer
add comment

I personally use regions as a way to group various types of methods or parts of code together.

So a code file might look like this upon opening it:

  • Public Properties
  • Constructors
  • Save Methods
  • Edit Methods
  • Private Helper Methods

I do not put regions inside of methods. IMHO that is a sign of code smell. I once came across a method that was over 1200 lines long and had 5 different regions in it. It was a scary sight!

If you are using it as a way to organize your code in a way that will make finding things faster for other devs, I don't think it's a sign of trouble. If you are using it to hide lines of code inside of a method, I'd say it's time to rethink that method.

share|improve this answer
6  
Ugh. I know this is a subjective topic, but man, I really can't stand this scheme. In my experience the added 'organization' doesn't help at all and it just makes browsing the code a pain in the neck. I also prefer grouping methods not only by access modifier but by logical responsibility. For the public interface I typically group each method/property together, but often times a protected method may call a private helper function, and in that case I much prefer the helper function (which may only be used there) to be above or below the method that calls it. –  Ed S. Feb 28 '11 at 23:32
show 2 more comments

On a recent project, there was a 1700 line method with several regions embedded in it. The interesting thing is that the regions demarced distinct actions that were being done within the method. I was able to do a refactor -> extract method on each of the regions without impacting the functionality of the code.

In general, regions used to hide boiler plate code are useful. I would advise against using regions to hide properties, fields, and the like because if they are too unwieldy to look at when working within the class, it's probably a sign that the class should be further broken down. But as a hard rule, if you're putting a region within a method, you'd probably be better off extracting another method that explains what is happening than wrapping that block in a region.

share|improve this answer
add comment

My rule of thumb is: If you have more than 5 regions in a file it's a code smell

Ie, it might be fine to delineate field,methods, properties and constructors, but if you're starting to wrap every other method in a region of it's own something is seriously wrong

..and yes, I've been on plenty of projects where that's the case, often because of poor coding standards, code generation or both. It get's old fast having to toggle all outlining in visual studio to get a good overview of the code.

share|improve this answer
add comment

REGIONS HAVE THEIR USE

I've used them personally for "hand coding" interface events before for windows forms apps.

However at my work we use a code generator for handling SQL and it automatically uses regions to sort out its select, update, delete, etc types of methods.

So while I don't use them often, they're perfectly fine for removing large chunks of code.

share|improve this answer
show 2 more comments

Can regions be used in code which is good quality? Probably. I bet they are, in many cases. However, my personal experience leaves me very suspicious - I have seen regions almost exclusively misused. I'd say that I'm jaded, but still optimistic.

I can roughly divide the region-using code I've seen to date into three categories:

  • Poorly factored code: Most of the code that I've seen uses regions as a poor-man's factoring tool. For example, a class which has grown to the point where it makes sense to specialize it for different purposes might be instead split into separate regions, one for each purpose.

  • Code written using the wrong libraries, and sometimes the wrong language, for the problem domain Often, when a programmer is not using the right set of libraries for the problem domain, you'll see code becoming incredibly verbose - with lots of little helper functions which really don't belong (they probably belong in their own library).

  • Code written by students or recent graduates. Some programs and courses seem to try and inculcate students with the use of regions for all sorts of strange purposes. You'll see regions littering the source code to the point where the ratio of region tags to lines of code is in the range of 1:5 or worse.

share|improve this answer
add comment

I would say it is a "code smell."

Anti-patterns are generally fundamental structural problems in a piece of software, whereas regions, by themselves just cause an obnoxious behavior in an editor. Using regions isn't actually inherently bad, but using them a lot, especially to hide chunks of code can indicate there are other, independent and greater problems going on elsewhere.

share|improve this answer
show 1 more comment

I use regions for one thing only (at least I can't think of other places I use them): to group unit tests for a method.

I usually have one test class per class and then group the unit tests for each method by using regions that have the name of the method. Not sure if that's a code smell or anything, but since the basic idea is that unit tests don't need to change unless they break because something in the code changed, it makes it easier for me to find all the tests for a specific method pretty quickly.

I may have used regions to organize code in the past, but I can't remember the last time I did it. I stick to my regions in unit test classes though.

share|improve this answer
show 2 more comments

I mainly use regions for CRUD server classes to organize the various types of operations. Even then, I could gladly go without them.

If used extensively, it would raise a red flag. I would be on the lookout for classes that have too much responsibility.

In my experience, a method with hundreds of lines a code is definitely a smell.

share|improve this answer
add comment

The key word here is "judicious". It's hard to imagine a case where putting a region inside a method is judicious; that is all too likely to be code-hiding and laziness. However, there can be good reasons for having a few regions here and there in one's code.

If there are lots and lots of regions, I do think that is a code smell. Regions are often a hint at a possible place for future refactoring. Lots of regions means someone isn't actually ever taking the hint.

Used judiciously, they give a good middle-ground between the structure of a single class with many methods and the structure of many classes with just a few methods in each. They are most useful when a class starts to get close to the point where it should get refactored into multiple classes, but isn't quite there yet. By grouping related methods together, I make it easy later on to extract a set of related methods into their own class if they continue to grow in number. E.g., if I have a class that is approaching 500 lines of code, that set of methods using 200 lines of code total collected together in a region is probably a good piece to refactor somehow - and that other region with 100 lines of code in its methods might also be a good target.

Another way I like to use regions is to reduce one of the negative effects of refactoring a large method: Lots of small, concise, easily reused methods that a reader has to scroll through to get to another mostly-unrelated method. A region can be a nice way to meta-encapsulate a method and its helpers for the readers, so someone who is working with a different aspect of the class can just collapse them and quickly dismiss that part of the code. Of course, this only works if your regions are really well organized and essentially being used as another way to document your code.

In general, I find regions help me keep myself organized, help "document" my code, and help me catch places to refactor much sooner than if I don't use regions.

share|improve this answer
add comment

If you have regions IN code you certainly have a problem (barring the case of generated code.) Putting regions in code is basically saying "refactor this."

However, there are other cases. One that comes to mind that I did a while back: A table with a couple thousand precalculated items in it. It's a description of geometry, short of an error in the table there will never be an occasion to look at it. Sure, I could have obtained the data from a resource or the like but that would preclude using the compiler to help make it easy to read.

share|improve this answer
add comment

I use regions to contain each combination of visibility and member type. So all private functions go into a region, etc.

The reason I do this is not so I can fold up code. It is because I have my editor scripted up so I can insert, say, a reference to a proxy:

#region "private_static_members"
 /// <summary>
 /// cache for LauncherProxy
 /// </summary>
private static LauncherProxy _launcherProxy;
#endregion

#region "protected_const_properties"
protected LauncherProxy LauncherProxy{
  get{
    if(_launcherProxy==null) {
      if (!God.Iam.HasProxy(LauncherProxy.NAME)) {
        God.Iam.RegisterProxy(new LauncherProxy());
      }
      _launcherProxy=God.Iam.Proxy(LauncherProxy.NAME) as LauncherProxy;
    }
    return _launcherProxy;
  }
}
#endregion

into the code and have each part neatly tucked into the proper region.

In this case the macro would analyze my project, give me a listbox of proxies and inject the code for the one I want. My cursor doesn't even move.

In the beginning of learning C# I had considered the use of regions for keeping commonality together, but that is a hit and miss proposition because its not a one-to-one relationship at all times. Who wants to fret over a member used by two regions, or even begin to break things up on those terms.

The only other type of segregation is methods- I will break out methods into Commands, Functions, and Handlers, so I would have a region for public, private, etc Commands, etc.

This gives me granularity, but it is consistent, unequivocal granularity I can rely on.

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.