It's not a code smell, nor an anti-pattern. But it's clearly a bad practice.
Regions inside methods are bad
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.
Regions outside methods are bad
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.
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's a good practice. StyleCop SA1124 rule is clear: you should not use regions. Never.
I'm currently doing a code review of my coworker. 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 unmaintainable 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.