Why are people so strongly opposed to #region tags in methods?

Question

I hear a lot about keeping methods short and I've heard a lot of programmers say that using #region tags within a method is a sure sign that it is too long and should be refactored into multiple methods. However, it seems to me that there are many cases where separating code with #region tags within a method is the superior solution to refactoring into multiple methods.

Suppose we have a method whose computation can be separated into three rather distinct phases. Furthermore, each of these stages is only relevant to the computation for this method, and so extracting them into new methods gains us no code reuse. What, then, are the benefits of extracting each phase into it's own method? As far as I can tell, all we gain is some readability and a separate variable scope for each phase (which will help prevent modifications of a particular phase from accidentally breaking another phase).

However, both of these can be achieved without extracting each phase into its own method. Region tags allow us to collapse the code into a form which is just as readable (with the added benefit that we no longer have to leave our place in this file should we decide to expand and examine the code), and simply wrapping each phase in {} creates its own scope to work with.

The benefit to doing it this way is that we don't pollute the class level scope with three methods which are actually only relevant to the inner workings of a fourth method. Immediately refactoring a long method into a series of short methods seems to me to be the code-reuse equivalent to premature optimization; you are introducing extra complexity in order to address a problem which in many cases never arises. You can always extract one of the phases into its own method later should the opportunity for code reuse arise.

Thoughts?

Answer 1

All you should ever care about is for your code to be usable, not reusable. A monkey can transform usable code to reusable code, if there are any transformations to be done at all.

The argument "I need this only here" is poor, to put it politely. The technique you're describing is often referred to as the headlines technique and is generally frowned upon.

1. You can't test regions, but you can test true methods in isolation.
2. Regions are comments, not syntactic elements. In the worst case the nesting of your regions and your blocks contradict each other. You should always strive to represent the semantics of your structure with the syntactic elements of the language you are using.
3. After refactoring to methods, one no longer needs folding to read the text. One might be looking at source code in any tool that doesn't have folding, like a terminal, a mail client, a web-view for your VCS or a diff-viewer.
4. After refactoring to methods, the resulting method is at least as good as with regions, plus it is simpler to move the individual parts around.
5. The Single Responsibility Principle suggests that any unit should have one task and one task only. The task of the "main" method is to compose the "helper" methods to get the desired result. The "helper" methods solve a discrete, simple problem in isolation. The class containing all these methods should also only fulfill one task and only contain the methods related to that task. So the helper methods either belong into the class scope, or the code shouldn't be in the class in the first place, but at least in some global method or, better yet, should be injected.

Also relevant: Jeff Atwoods's thoughts on code folding

Answer 2

It's not the regions in the methods that are the problem, but the case when there's a need (or even a use) to put regions in methods.

Having had to debug plenty of 200-300 line methods, I can tell you that I wouldn't wish it on anyone. If you're writing and looking after your own code that's fine - do whatever you want. But once someone else looks at it, it should be immediately clear what a method is doing.

"First it gets the things, and then it jigs them around, then if they need buzzing it does that, otherwise it checks whether they are blue and inverts their Copernicus value. Then, if the second thing is greater than the first thing, we need to get the juice box and insert it... "

No, that is not good enough. Break it up into regions all you want but I'm still shaking my head just thinking about it.

If you break into methods you get many benefits:

• Clearer understanding of what's happening where, even if just through the name of the method. And you are wrong that a method can't be fully documented - add the documentation block (hit ///) and enter the description, parameters, return type, and also exception, example, remarks nodes to detail those scenarios. That's where it goes, that's what it's for!
• There are no side effects or scoping problems. You may say that you declare all your variables in a sub-scope with {}, but do I have to check every method to make sure you didn't 'cheat'? Is this dodgyObject I'm using here only because used in this region, or did it come from somewhere else? If I were in my own method I'd be easily able to see whether it was passed to me or whether I created it myself (or rather, whether you created it).
• My event log tells me which method an exception occurred in. Yes, I may be able to find the offending code with a line number, but knowing the method name (especially if I've named them properly) gives me a very good indication of what happened and what went wrong. "Oh, the TurnThingsUpsideDown method failed - it's probably failing while turning a bad thing" is a lot better than "Oh, the DoEverything method failed - it could have been 50 different things and I'll have to dig around for an hour to find it".

This is all before any reusability or improvability concerns. Properly separated methods obviously facilitate reuse, and also allow you to easily replace a method that's not performing (too slow, too buggy, changed dependency etc). Have you ever tried refactoring any of these huge methods? There's no way to know that what you're changing won't effect anything else.

Please properly encapsulate your logic into reasonably sized methods. I know that it's easy for them to get out of hand, and arguing that it's not worth redesigning once at that point is another issue. But you must accept the fact that there is no harm and at least potential benefit in having properly encapsulated, cleanly written and simply designed methods.

Answer 3

If your method is complex enough that it can be separated into three distinct phases, then not only should those be three separate methods...but your method should in fact be a separate class, dedicated to that computation.

"But then my class will have only one public method!"

You're right, and there's nothing wrong with that. The idea of the single responsibility principle is that your class does one thing.

Your class can call each of the three methods in turn, and return the result. One thing.

As a bonus, now your method is testable because it's not a private method anymore.

But never mind one class; in reality you should have four: One for each of the pieces of your method, plus one that takes each of the three as a dependency and calls them in the proper order, returning the result.

This makes your classes even more testable. It also makes it easy for you to change one of those three pieces. Just write a new class inheriting from the old one, and override the changed behavior. Or create an interface for the behavior of each of your three pieces; and then to replace one, just write a new class implementing that interface and substitute it for the old one in your dependency injection container configuration.

What? You're not using dependency injection? Well, you probably haven't seen the need yet, because you're not following the single responsibility principle. Once you start, you'll find that the easiest way to give each class its dependencies is to use a DI container.

EDIT:

OK, let's set aside the refactoring question. The purpose of #region is to hide code, which mainly means code that doesn't need to be edited under any normal circumstances. Generated code is the best example. It should be hidden in regions because ordinarily you don't need to edit it. If you do need to, then the act of expanding a region gives you a bit of "Here be dragons"-style warning to make sure you know what you're doing.

Therefore I would say #region should not be used in the middle of a method. If I open a method, I want to see the code. Using #region gives me another level of hiding that I don't need, and that becomes an annoyance: I unfold the method...and still can't see any code.

If you're worried about people seeing the structure of the method (and you refuse to refactor it), then add banner comments like this:

//
// ---------- COMPUTE SOMETHING OR OTHER ----------
//

These will help someone browsing the code see the individual parts of your method without having to deal with expanding and collapsing #region tags.

Answer 4

I think that the example you give in your argument is less about YAGNI (You ain't gonna need it) and more about writing proper quality code that is easily maintainable.

In the earliest programming courses we learn that if a method is 700 LOC long, it is probably too big and certainly would be a giant pain to try and debug.

Secondly if I write methods A, B and C that are only used within method D, then I can more easily unit test A B and C independently of D, allowing for more robust unit tests in all 4 methods.

Answer 5

Suppose we have a method whose computation can be separated into three rather distinct phases. Furthermore, each of these stages is only relevant to the computation for this method, and so extracting them into new methods gains us no code reuse. What, then, are the benefits of extracting each phase into it's own method? As far as I can tell, all we gain is some readability and a separate variable scope for each phase (which will help prevent modifications of a particular phase from accidentally breaking another phase).

Those are two benefits. But the important one, to me anyway, is debuggability. If you have to trace through that code, it's a lot simpler to be able to Step Over two of the three sections and only trace into the one that you care about. Refactoring into smaller methods allows this; region tags do not.

Answer 6

My personal rule is that if a method is longer than one screen, say 40 lines, it is too long. If a class is longer than 500 lines, it is too big. I break these rules sometimes, occasionally even by a large multiple, but I generally regret it within the year.

Even a method that is 20 to 30 lines is getting a bit long in most cases. So I would say using regions in a method is usually a bad idea, simply because it would almost never make sense to collapse a region of 20 to 30 lines into multiple sub-regions.

Breaking down functions that are long by this definition has at least two benefits:

1. You get to put a name on what those sub-functions are doing, which clarifies your present thinking and simplifies the task of later maintainers.

2. Decomposing into smaller functions forces you to pass only the parameters those smaller functions need, so the scope of the data they require and modify is very clear. This assumes you aren't accessing and modifying object state in a hundred different leaf functions, which I would also discourage.

In my experience, these rules produce code that is easier to write without making common mistakes and can be understood and modified later without breaking subtle behaviors. It doesn't matter if the person who has to understand/modify the code is someone else, or myself after I have slept and forgotten everything.

So I would consider regions in methods to be a "code smell" indicating that unsustainable practices are being applied. As always, there could be exceptions, but they occur so infrequently that they almost aren't worth discussing.

Answer 7

Here we go again ... There are plenty of other topics here on programmers which have ended up in the same discussion.

You point out some very interesting arguments relating to short functions. It is not a popular statement, but I am with you on this. After having discussed it oft times before, it is my impression the discussion usually boils down to whether you primarily focus on proper encapsulation, or take test driven development to the max. These are the two main contenders in what seems to be turning out yet another holy war, and I'm afraid we are on the under powered side.

However ...

The solution in my opinion definitely isn't wrapping the 'phases' in regions. Code folding is used to sweep code under the rug. Leave the code there as it is, it might remind you that you might want to refactor that at a later time! To relate it to an argument in your question: how will you be able to see an opportunity for code reuse if you don't see any code? Long segments of code should be exactly that, a thorn in the eye which makes you want to refactor it.

As an appropriate replacement for regions, I suggest using code paragraphs as Alex Papadimoulis names them in a comment. You might actually already be using a similar approach. It are simply blocks of code with a comment header, explaining what the entire block does. You have the readability of functions, but are able to use normally spaced english sentences, and you don't lose any encapsulation.

Answer 8

Although I agree that usually it's better to refactor code than to use #region, it's not always possible.

To support that statement, let me give an example.

class Foo : IComparable
{
  private String name;
  private int foo;
  private Bar bar;

  public int CompareTo(Foo other)
  {
#region Ascending on name
     if (this.name < other.name) return -1;
     if (this.name > other.name) return 1;
#endregion
#region Usual order on bar
     int compBar = bar.compareTo(other.bar);
     if (compBar != 0) return compBar;
#endregion
#region Descending on foo
     if (this.foo > other.foo) return -1;
     if (this.foo < other.foo) return 1;
#endregion
     return 0; // equal
  }

It's easy to readjust the regions to alter the order, or to extend when addional fields are added to the class. Comments are required anyway to quickly see how the ordering is supposed to work. And most of all: I don't see how refactoring will help readability in this case.

However, those exceptions are rare. Usually it is possible to refactor, like many of the other answers say.

Answer 9

I don't think there is a single answer for why certain people or teams decide not to use #region but if I had to list a few this would be the top reasons that I've seen.

• The names and order of the regions make more explicit the ordering and organization of code which enforces a particular style which can become a source of contention and bikeshedding.
• Most concepts don't cleanly fit into a small number of regions. Typically you start with regions by access modifiers public, private then maybe by member type (ex. method, property, field) but then what about constructors that span those modifiers, static varieties of all of these, protected members, internal members, protected internal members, implicit interface members, events, etc. In the end you would have most well designed classes where you have a single or method in each region due to the granular categorization.
• If you are able to keep pragmatic and only group things into a say three or four constistent types of regions then that may work but what value does it really add? If you are designing classes well you really shouldn't need to sift through pages of code.
• As hinted above regions can hide ugly code which may make it seem less problematic than it is. Keeping it as an eye sore may aid in getting it addressed.