Return to Answer

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.

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.

It is not 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.

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.

It's not a code smell, nor an anti-pattern. But it's clearly a bad practice.

Regions inside methods are bad

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.

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:

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.

It is not 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

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.

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.

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:

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.