Sysadmin Field Notes

August 9, 2006

I've lightly covered regions before but since Google has no appropriate entry for "Regions considered harmful" Neil suggested that I write one.

First the background, what's a region? As specified in the grammar, regions are pretty innocuous. They have no semantic meaning, the simply identify a region of code. Seems easy enough right?


#region some hunk of code
foo = this.bar();
bar = new FooBar();
#endregion


The default behavior of Visual Studio is to collapse these regions so the code under them is not visible. People use them to surround public methods, private variables, or regions of interesting code, etc... It's often used to make long classes "more readable" because now the whole class fits on a single screen by default. Why are the bad? One simple reason.

Regions encourage longer and more complex methods and classes

The class or method doesn't really fit on one page, it's just collapsed. It still might be 5000 lines. What's more, is that you have to hunt around in the regions to find out where the real complexity might be. Or just expand them all. Then you realize you're dealing with a 5,000 line class that desperately needs refactoring.

This goes double for methods. I've seen things like this:


if (thing.ShouldHaveFoo()) {
thing.DoThingWith(x);
[do more]
}


(the brackets are my attempt to duplicate the little box and + sign visual studio puts around regions when they are collapsed)

That looks very simple and easy right? But what happens if you expand the Do More region and find that it's actually 5 levels of nested loops and case statements? That code is still there, it's just hidden. I've seen even more pathological examples where the regions span portions of nestings, or there are several regions to hide the innermost bits of a loop.

None of this removes complexity. It just hides its. Like a rug you can sweep dirt under. That's not a great analogy though, because I don't really care if there's dirt under my rug, but I do care if there's hidden complexity in my programs.

Bottom line is that if you can semantically define a set of operations with a logical name that makes sense to people, make it a method. Now it can be re-used, examined, debugged more obviously. If you say "calling a method here would be too expensive," then you better have a comment above the region indicating so.

What you're really doing when you mix regions into complex code is probably mixing abstraction layers, which is bad. Since there's no scoping, it's very easy to use variables outside of the region, which is going to be very confusing to anyone trying to read the code with outlining enabled.

Since outlining (collapsing regions) is enabled by default, that means most programmers will miss this hidden complexity. It's very easy to miss a collapsed region. I've talked to people who have been unable to find a bug, because they didn't notice that half the code was hidden in one line in an unexpected region. So if everyone just has to disable outlining by default or turn it off to read your class, why add regions in the first place?

Let me handle some of the most likely justifications for regions.

Aren't regions useful for hiding generated code?

I would generally say this is probably okay, but wouldn't you rather have the generated code completely separate? I'd rather have whole files marked "off limits" to prying eyes, since life is easier if you can just regenerate whole files.

ASP.net 1.1 uses regions around the designer-introduced code for forms, but they have since moved to partial compilation, which as I understand it keeps the generated code out of your files. Much preferred. So with generated code, regions are just hiding things in a less than ideal way.

Regions are handy for marking the public API of a class

Here I'd probably agree with you, although I've never seen anyone do this before. And it's not terribly useful. If I just want to see the public API, then Visual Studio or Resharper is going to show me that. I'm probably looking at the class because I want to see the implementation, and that just means I'll be expanding a whole bunch of regions.

Typically people have regions like "public methods", "public properties", "private methods", "internal methods", etc...Who cares? If I've resorted to looking at the implementation of a class in my project, it's because I need to see how it's implemented. I certainly am not interested in the distinction between public methods and public properties.

So let's recap:

1. You can't automatically refactor a region,
2. Regions don't abstract complexity away, they just hide it,
3. they encourage longer classes/ethods since it can "look" like the class or method is shorter.

And on the pro-side, they might occasionally be useful for marking the public api of a class, but that's not really necessary all that often. And besides, everyone's going to be turning off regions altogether (hopefully), so they won't see the hiding anyway.

So anytime you go to write a #region, think of it as a poor man's method or subroutine, do yourself a favor and stop. Actually abstract out that functionality into a method with a clear interface. Maintenance programmers everywhere will thank you.

Bonus tip to disable outlining in VS 2003 forever:

First, make sure you only have one copy of Visual studio running (last one you close wins the preferences battle). Go to Tools->Options. Select Text Editor->C#. Unselect "Enter outlining mode when files open". Now exit visual studio to make sure it saves, and you never have to expand a region again.