305

C# allows the use of #region/#endregion keywords to make areas of code collapsible in the editor. Whenever I do this though I do it 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 so to me.

eebbesen
  • 137
  • 1
  • 8
Craig
  • 4,302
  • 3
  • 21
  • 21
  • 10
    FYI: CodeMap pretty much eliminates the needs for regions. http://visualstudiogallery.msdn.microsoft.com/1c54d1bd-d898-4705-903f-fa4a319b50f2 - 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. –  Feb 28 '11 at 23:11
  • 7
    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
  • 9
    For what it's worth, Jeff Atwood [hates them](http://www.codinghorror.com/blog/2008/07/the-problem-with-code-folding.html). He argues that they hide bad code. – Brian Jun 05 '12 at 18:11
  • Related question on SO http://stackoverflow.com/questions/755465/do-you-say-no-to-c-sharp-regions – Michael Freidgeim Mar 29 '13 at 18:38
  • 6
    Code smell is a developer who don't shower and don't use a deodorant! – marko Aug 19 '13 at 16:29
  • 5
    Carefully crafted Regions in stinky code is what a few squeeze of Febreeze does on a crusty couch. It makes it bearable until you find the (time) money to replace it. – Newtopian Aug 19 '13 at 17:27
  • What about hidding helper methods? Mike uses regions in his wonderful Export to Excel 2007 code http://mikesknowledgebase.azurewebsites.net/pages/CSharp/ExportToExcel.htm – Broken_Window Dec 03 '14 at 23:03
  • [Related](http://staticola.com/Blog#on_code_regions) – Robbie Dee Jul 25 '16 at 22:11
  • bit of a loaded question, leaves no room for being in defence of regions. – Newtopian Jun 02 '17 at 17:22
  • VS2017 addon supercharger, works quite nice with regions. In coding #region makes no difference, but in CodeMap navigation quite usefull. – Peter Jan 17 '18 at 12:55
  • 1
    There is a religious quality to the region hating answers. "You should use partial classes!" Note there is no fundamental difference between regions used to separate class members and partial classes. They serve the exact same purpose. The only difference is regions are more powerful in that purpose: you can apply them on a finer grained level than partial classes if you wanted to. Whether you should want to apply them in any given scenario is a separate question. "This has the capability to be abused so it sucks". Really? Anything that does not have the capability to be abused is useless. – Martin Maat Oct 01 '19 at 05:45

19 Answers19

331

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.

Arseni Mourzenko
  • 134,780
  • 31
  • 343
  • 513
  • What makes property groupings "StyleCop" compliant? – James Jun 05 '12 at 21:36
  • 17
    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 06 '12 at 08:24
  • 12
    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
  • 51
    @radarbob: in short, regions are helpful in crappy code which shouldn't exist in the first place. – Arseni Mourzenko Mar 28 '13 at 08:35
  • 53
    -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 Saeeduddin Aug 01 '13 at 20:26
  • 2
    @Asad: Why are you collapsing all properties individually? I would suggest to you to start using an IDE like Visual Studio which does that for you, and my answer covered only the situations where such IDE is used. – Arseni Mourzenko Aug 01 '13 at 21:32
  • @MainMa I'm already using Visual Studio (Express though, so maybe I'm missing some features). I can't collapse groups of class members, only individual class members, blocks inside them, or the whole class. – Asad Saeeduddin Aug 01 '13 at 21:38
  • @Asad: Strange. Try Ctrl + M, O (or right click → Outlining → Collapse to definitions). I would be surprised if Visual Studio Express doesn't have this feature. – Arseni Mourzenko Aug 01 '13 at 21:46
  • 3
    @MainMa I think I see what you mean, `Ctrl + M, O` collapses **everything** (recursively no less). I thought you were describing a way I could fold all fields into a single group, properties into another, and so forth for methods, internal types etc. Folding and unfolding everything simultaneously isn't very useful to me, since the point is to have a "region" of code that I'm focusing on front and center and other groups of interest neatly tucked away, only a *single* click away. – Asad Saeeduddin Aug 01 '13 at 21:53
  • @Asad: so you want to collapse only your 10-12 properties (which means 29-35 LOC including whitespace). Question: why? Won't you spend more time collapsing and uncollapsing your region then scroll up and down those 35 LOC? – Arseni Mourzenko Aug 01 '13 at 22:01
  • 2
    @MainMa I actually work mostly with VB, and can't fit getters, setters elegantly onto a single line as in C#. That means 10-12 properties (all of which are usually Readonly, and therefore not autoimplemented) take up about a hundred lines of code. – Asad Saeeduddin Aug 01 '13 at 22:07
  • @Asad: I'm not sure we are talking about the same thing. I'm not talking about auto-implemented properties, but properties with a fully-implemented custom getter and setter, collapsed to two LOC (one for the property header, another one for XMLDoc) with the help of Visual Studio. According to [this screenshot](http://diditwith.net/content_images/vb_property_collapse_1.png), you can do that. – Arseni Mourzenko Aug 02 '13 at 06:18
  • @MainMa: What about ASP.NET WebForms and generated method InitializeComponent() - you cannot move it safely to another partial class, so best you can do is to surround it with regions (containing text generated code). For example to ensure not being processed by StyleCop/FXCop/Sonar. – Pavel Hodek Aug 27 '13 at 12:39
  • 1
    One acceptable case for regions I've found--when you have a whole block of data embedded in your code. Who needs to look at that table with all possible permutations of a set of values? – Loren Pechtel Jun 16 '15 at 03:13
  • 21
    I Strictly disagree with this answer: Code stink and region have nothing to do with each other. If your code stinks, it would stink with or without regions. The way I use regions is to segregate my classes into regions. Generally I stick to the same pattern though: Public Properties, Public Methods, Private Fields, Private Methods. That is the only use I would have regions for. Anything else, you are probably breaking SRP principal in your code. – Alexus Jun 23 '15 at 20:19
  • This convinces me, I'll replace my field/constructor/method grouping regions with comment decorations like `//---- getters -----------`. At least I can't fold them, which forces me to see how big a class becomes. – Domino Aug 18 '15 at 13:50
  • 3
    @MainMa re: **regions are helpful in crappy code which shouldn't exist in the first place** - Which means my primary use is when working with old code that has evolved over decades into a bloody mess. I group things as I read to help me understand this old code, which I must do before it is safe to refactor. But you are right - `well written code that follows the standards of style` does not need regions. Sadly, for old code, that is a rare situation. – Jesse Chisholm Aug 26 '15 at 16:11
  • 3
    I disagree with that one should extract methods instead of creating regions in existing method. I think in OOP you shouldn't have unnecessary private methods as it's more a part of functional programming. It's better to make a class OR [extract methods *only if they gonna be REused*]. There is no sense to have one more method if it's called from only one place and you have to navigate there to understand what it does so I prefer scopes and regions when it's not too nested. (But there *is* a sense to have a class even if it's used only in one place because you can substitude implementation). – Vlad Feb 15 '16 at 16:13
  • 3
    @Vlad if *you have to navigate there to understand what it does*, then you need better method names. *Abstraction* is also one of the 4 pillars of OOP AFAIK. – Mathieu Guindon Feb 18 '16 at 18:11
  • @Mat'sMug ok, you are right about names but anyway if you need to open it you have to jump may be a few screens below... If it's called only one time I would not extract a method in such case. – Vlad Feb 18 '16 at 22:34
  • @Vlad that's Ctrl+Click in my IDE ;-) – Mathieu Guindon Feb 18 '16 at 22:36
  • @Mat'sMug 1: you will still leave your current context in the source. 2: abstraction with methods is a part of functional programming; in OOP we use *classes for abstraction*. – Vlad Feb 18 '16 at 22:39
  • 10
    This answer would be valid if we used a brain-dead simple text editor for code files, not advanced IDE like Visual Studio with possible additional add-ons and tools for code folding and navigation. It doesn't matter I have my 50 events grouped all together. When I work on internal handlers I don't want to see them in code. Regions improve readability hugely in this case. Regions CAN be anti-pattern as everything: IF USED INCORRECTLY. If the author can't find the correct use for regions it doesn't mean it doesn't exist. – Harry Apr 03 '16 at 10:47
  • I feel this answer applies to 99% of the time for 99% of circumstances, but I think it's heavy-handed to say "never". For example, Suppose I'm re-writing the `DateTime` class (actually I am, but not in C#). There are hundreds of fields and functions, and inheritance isn't really a good solution in this case. Most of my code recently is actually in complex mathematical optimisation heuristics, and the functions are often longer and more complex than those typically found for a web or desktop application. Long, complex functions with many parameters in several highly-coupled stages. – Ozzah Mar 31 '17 at 04:56
  • @Ozzah: so you are suggesting to cram hundreds of lines of complex code in a single method, and then use regions to make it *look all right*? Wow. Hopefully I'll never have to be that poor guy who maintains your code. BTW, I already addressed this point in the *very first* part of my answer. Also, feel free to check yourself the *actual* source code of the `DateTime` class. How many regions do you see? Right, zero. And no “long, complex functions with many parameters in several highly-coupled stages” either. – Arseni Mourzenko Mar 31 '17 at 20:13
  • @ArseniMourzenko No need to be rude. I never said `DateTime` had long complex functions with many parameters; I said some parts of my code does (high performance scientific computing code run in competition scenarios). I said `DateTime` had a large number of functions - which it does. You could, for example, put the operator overloads in a region, the getters/setters in one region, the formatters in one region, etc. No need to scroll up and down 1000 lines of code when only working on a very small subset. As I said, I 100% agree with what you said 99% of the time. There are always edge cases. – Ozzah Apr 02 '17 at 04:05
  • 1
    Is there a good use for regions? Yes, e.g. in order to separate members of an implicit interface implementation in C#. That may help other people understand _faster_ how a type is organized. – Marc Sigrist Aug 30 '17 at 15:05
  • A good use for a #region directive is to hide away a long block of documentation (like a copyright notice) in the top of a class. – T. Sar Nov 20 '17 at 16:25
  • While I agree with this post, when working in VS Professional/Enterprise and CodeLens is on, that adds a lot of extra white space to properties. Being able to collapse those properties (say 20 properties takes up ~40 lines with CodeLens on) saves me from scrolling so much. – SaoBiz Jan 10 '18 at 00:21
  • All these comments made for an interesting read. I agree that regions should not be necessary in clean code. I've also seen the way it completely destroyed the readability of code. And I've seen bugs and even compile-time errors caused by region use. All in all, this made me write a rule in our coding guidelines, banning regions from regular code. The one exception is generated code, though. – Jonathan van de Veen Feb 08 '18 at 12:22
  • 1
    Why call it "InternalFoo", instead of "PrivateFoo"? – AustinWBryan May 20 '18 at 06:28
  • @AustinWBryan I think perhaps because they are both private methods, but "internal" tells you that it is called only from "Foo". But I don't know. – Buttle Butkus Mar 01 '19 at 19:28
  • @ButtleButkus That would be very misleading if that was the case. – AustinWBryan Mar 08 '19 at 01:23
  • @AustinWBryan well it would be a way to organize a class with a lot of non-public methods into groups. Of course, the code could be refactored so that each group of methods is its own class, perhaps. “Internal” is a weird choice of word, though. – Buttle Butkus Mar 08 '19 at 01:26
  • @AustinWBryan: as stated in my answer, this was the choice which was made by .NET Framework developers. Only them would be able to answer your question; anything else is only speculation. – Arseni Mourzenko Mar 08 '19 at 18:18
  • I disagree with this answer. There are few absolutes in the universe. Like others have said with WPF, the same applies, for me at least, to large MVC projects. You either have endless controllers, or large controllers with a lot of code #regioned up. I don't like the MVC pattern a single iota; however when you have to use it, regions can come in handy. – Mmm Nov 23 '22 at 23:33
  • I disagree. Bad code is bad whether or not regions are involved, and I can destroy my project's readability misusing *any* organizational feature. There is nothing special about regions in that. That said, regions do provide a novel way to segregate your code *without adding otherwise useless dependencies*. How far will you take SRP? Will you write a class *just* to handle resource disposal? The full async disposable pattern can add a lot of lines, and when you do not need to see them, **you do not need to see them**. Regions eliminate that noise without the cost of dependencies. – Daniel Mar 03 '23 at 19:11
136

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.

Cody Gray - on strike
  • 1,737
  • 2
  • 19
  • 22
  • 12
    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 01 '11 at 06:50
  • 33
    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 01 '11 at 06:57
  • 4
    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 01 '11 at 07:50
  • 6
    @Kristof: As are properties in .NET 4.0 that do simple input validation. Automatic properties just haven't been all that magical for my purposes. – Cody Gray - on strike Mar 01 '11 at 07:53
  • 9
    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. – TtT23 Nov 28 '12 at 07:53
  • 1
    It seems to me that the primary arguments for splitting methods and classes *finer than semantics would suggest* stem from the historical inability of development tools to work with larger methods and classes effectively. If a piece of code is only ever used once, within a one particular loop, pulling it into its own method will often make it easier to see and understand the loop as a whole, but harder to understand that code's role within the loop. Using `#region` to collapse the code when looking at the loop outside it and expand it when looking at the code itself would be better, IMHO. – supercat Feb 02 '15 at 19:42
  • 1
    I totally agree with Kirk. I just wrote a WPF control with about 16 *completely-related and inseparable* controls with backing properties. Each property is *almost* an auto-property, but the setter needs to have an `if (_backingField != value) { _backingField = value; OnPropertyChanged(); }`, and these 16 fields end up taking 13 lines each uncollapsed + whitespace between the fields. I put these in a region and collapsed it because these properties need never be touched, and are really just clutter on my screen. –  Feb 18 '16 at 18:19
  • 2
    Those of you who say regions are good for hiding WPF backing properties...why can't you put those properties in a partial class? – Kyralessa Jul 25 '17 at 08:26
72

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 text 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. :)

Ed S.
  • 2,758
  • 2
  • 21
  • 24
  • 3
    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 01 '11 at 00:06
  • 7
    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 01 '11 at 00:11
  • 14
    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 01 '11 at 00:51
  • That's true by definition, but in practice it doesn't seem (to me at least) to be how it is used. – Ed S. Mar 01 '11 at 00:57
  • 7
    +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 01 '11 at 06:06
  • 1
    @Josh Einstein: Yeah, the use (or non-use) of regions is definitely subjective and there is no "right" way to do it. Whatever works for you and your team is the "right" way. As for standing up to "code smell"... I totally expected to be downvoted into oblivion =D – Ed S. Mar 01 '11 at 06:16
  • 1
    @Josh Einstein: Wait, that took a second to sink in: private methods are a "code smell"? WTF is that?! Honestly, the best coders that I have known have not been religious when it comes to things like this. Write code that is maintainable, extensible (as needed), and most importantly; works. The rest is just a waste of time. – Ed S. Mar 01 '11 at 06:18
  • 2
    I'm also +1'ing as I also can't stand "code smell". – Chris S Jul 13 '11 at 12:57
  • [This question](http://programmers.stackexchange.com/questions/133492/is-code-smell-still-a-useful-metaphor-or-has-misuse-of-the-term-subverted-its) might interest you, as your answer is referenced in it. – yannis Feb 06 '12 at 11:13
  • 7
    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 05 '12 at 20:22
  • 1
    This really should be the accepted answer, it provides a much more balanced argument. – Pharap Nov 18 '14 at 01:04
  • 1
    @EricKing sorry, but poor use of a metaphor does devalue it. At this point if someone says code smell to me i write them off. – Andy Aug 01 '15 at 20:31
  • 1
    @Andy I only write off the people who misuse the metaphor, or toss it out completely without consideration. Its proper use is powerful. – Eric King Aug 02 '15 at 03:13
  • 2
    @EricKing The problem is i can't remember the last time i've heard it used properly. Code smell directly translates to "i'm repeating something i read on the internet without really understanding it." – Andy Aug 02 '15 at 14:57
  • "It makes it harder to get at the code, and the code is what I am interested in" I am not, I am interested in the logic. This is exactly where regions come in handy, they hide away code from view to reveal logic. – Martin Maat Oct 01 '19 at 05:28
23

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 colleague has used some regions here!"?

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

I'd really care less if all my public methods 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!

Additionally if your file needs and 'information architecture' through use of regions you may want to combat the core problem: Your class is way too large! Breaking it up into smaller parts is much more beneficial and, when done properly, adds true semantics/readability.

Joppe
  • 4,548
  • 17
  • 25
  • Are #regions part of the compiler? I thought they were just a directive to the IDE that could be ignored. – Steve Rukuts Jul 13 '11 at 11:30
  • 2
    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 01:55
  • 3
    "Can you think of one example where you though "geez, I wish my colleague has used some regions here!"?" Yes, very much so. When I have a class with private methods and public methods, I break them up in regions because when refactoring the public methods, you don't necessarily need to touch the private code and vice versa. – Anshul Aug 27 '15 at 17:24
  • You've obviously never seen outsourced code. Sooo many times regions would have been great to see in outsourced code. Even though the code sucks, at least it would have been a lot easier to comprehend if it was group together in some logical sense. Although, if their code isn't logical chances are the regions wouldn't be either, so probably would have made it worse. Regions are great when used properly though. – Nickmccomb Aug 24 '17 at 02:30
15

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.

Tyanna
  • 9,528
  • 1
  • 34
  • 54
  • 16
    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
  • 3
    @Ed S. - that's why I said "might look like". It is very subjective. I'm not saying that every file should look like that. I'd be surprised if they did. Just an example on a complex topic. :) – Tyanna Feb 28 '11 at 23:48
  • 1
    Oh I know, like I said; it's subjective. Whatever works for you/your team. I just have it out for this scheme because it doesn't work (for me), but I had to maintain a project which did just this. It drove me crazy. – Ed S. Feb 28 '11 at 23:49
  • 4
    @EdS. So go into your options in vs and turn off regions. Problem solved. – Andy Aug 01 '15 at 20:33
  • Why do your objects have save methods? :( – TheCatWhisperer Mar 17 '17 at 18:55
  • I hate this with a passion as well! I'm in the same situation where I have to maintain a project where people went nuts with this. Every time I open a class, I have to expand about 5 to 7 regions just to see its dang private & public properties and constructor. Then expand its public methods, see they call private methods, uncollapse private methods, uncollapse a subregion within it for 'certain logic' etc. It's very tedious. – Zimano Sep 13 '21 at 09:31
12

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.

Austin Salonen
  • 1,061
  • 8
  • 11
6

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.

Ethel Evans
  • 5,329
  • 1
  • 23
  • 42
5

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.

Loren Pechtel
  • 3,371
  • 24
  • 19
  • 1
    That's a better use case for a partial class with that stored in a separate file, or perhaps an injected IMyDataSource with a HardcodedDataSource implementation. – Bryan Boettcher Jun 02 '17 at 20:05
5

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.

Homde
  • 11,104
  • 3
  • 40
  • 68
4

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.

TaylorOtwell
  • 2,657
  • 1
  • 21
  • 26
4

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.

Ken
  • 271
  • 1
  • 5
  • 1
    I have used similar code generators and prefer to use partial classes to remove generated code. – Craig Mar 01 '11 at 01:11
  • We do, the regions are inside the generated code to make it easier to read or debug (if needed). – Ken Mar 01 '11 at 01:24
3

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.

whatsisname
  • 27,463
  • 14
  • 73
  • 93
3

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.

Michael Brown
  • 21,684
  • 3
  • 46
  • 83
3

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.

blueberryfields
  • 13,200
  • 8
  • 51
  • 87
3

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.

Anne Schuessler
  • 522
  • 2
  • 6
  • 1
    Do you ever have tests that test more than one method? – Marcie Mar 02 '11 at 03:09
  • I don't really understand the question or what you're aiming it. The answer is: No, a unit test always is targeted at just one method or rather a certain aspect of one method. – Anne Schuessler Mar 02 '11 at 09:02
2

I believe the are a anti pattern and frankly think they should be eliminated. But if you are in the unfortunate situation of working in a place where they are a standard Visual Studio offers an awesome tool to minimize the amount you would like to vomit everytime you see a region I Hate #Regions

This plugin will max the font size of the regions really small. They will also be expanded so you won't have to hit ctr+m+l to open all regions. It doesn't fix this form of code cancer but it does make it bearable.

0

Regions are preprocessor expressions - in other words they are treated like comments and basically ignored by the compiler. They are purely a visual tool used in Visual Studio. Therefore #region is not really a code smell, because it's just not code. The code smell is rather the 800 line method which has many different responsibilities embedded within etc. So if you see 10 regions in one method - it's probably being used to hide a code smell. Having said that I have seen them used extremely effectively to make a class more pleasing to the eye and more navigable - in a very well written and structured class too!

BenKoshy
  • 119
  • 5
0

Regions were a nifty organizational idea, but failed to take into account some developer tendencies to want to overcategorize everything, and generally unnecessary according to most modern day OOP practices... they are a "smell", in the sense that using them often indicates that your class/ method is far too large and should be refactored, as you are likely violating the "S" of SOLID principles... but like any smell, it doesn't necessarily mean something is going bad.

Regions serve more purpose in functional code rather than object-oriented code, IMO, where you have long functions of sequential data that it makes sense to break up, but there have been times I've personally used them in c#, and they almost always focus on code you don't need to/ want to look at. For me, these were usually raw SQL string constants in the code base used for NPoco or its variants. Unless you actually care how the data comes to fill out the POCO object through your ORM, these were completely pointless to look at... and if you did care, hey, just expand the region and BAM! 150+ lines of some complicated SQL query for your viewing pleasure.

Jeremy Holovacs
  • 221
  • 1
  • 3
0

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.

Mark
  • 336
  • 1
  • 4
  • -1 as soon as I get 125 points to downvote. You are adding unneeded lines of code. WHY WHY WHY would you put a region around a property... if(God.Iam.AbusingRegions() == true) myName = "Mark" – DeadlyChambers Aug 04 '14 at 22:47
  • 1
    @DeadlyChambers The reason is stated in the second paragraph - I am using editor macros to inject common code patterns into the file, the regions help keep the file structured so similar items are grouped. I am not putting a region around a singular property, but all properties fall into a designated region depending on their attributes "protected_const_properties". Did you read the post?? – Mark Aug 05 '14 at 11:20
  • 1
    You can probably refactor that into: protected LauncherProxy LauncherProxy => God.Iam.GetOrAddProxy(ref _launcherProxy); and thus you now don't need region. Also _launcherProxy can be renamed into _launcherProxyCache so you don't need region nor comment there. – aeroson Jan 25 '17 at 08:37