Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

There are some (quite rare) cases where there is a risk of:

  • reusing a variable which is not intended to be reused (see example 1),

  • or using a variable instead of another, semantically close (see example 2).

Example 1:

var data = this.InitializeData();
if (this.IsConsistent(data, this.state))
{
    this.ETL.Process(data); // Alters original data in a way it couldn't be used any longer.
}

// ...

foreach (var flow in data.Flows)
{
    // This shouldn't happen: given that ETL possibly altered the contents of `data`, it is
    // not longer reliable to use `data.Flows`.
}

Example 2:

var userSettingsFile = SettingsFiles.LoadForUser();
var appSettingsFile = SettingsFiles.LoadForApp();

if (someCondition)
{
    userSettingsFile.Destroy();
}

userSettingsFile.ParseAndApply(); // There is a mistake here: `userSettingsFile` was maybe
                                  // destroyed. It's `appSettingsFile` which should have
                                  // been used instead.

This risk can be mitigated by introducing a scope:

Example 1:

// There is no `foreach`, `if` or anything like this before `{`.

{
    var data = this.InitializeData();
    if (this.IsConsistent(data, this.state))
    {
        this.ETL.Process(data);
    }
}

// ...

// A few lines later, we can't use `data.Flows`, because it doesn't exist in this scope.

Example 2:

{
    var userSettingsFile = SettingsFiles.LoadForUser();

    if (someCondition)
    {
        userSettingsFile.Destroy();
    }
}

{
    var appSettingsFile = SettingsFiles.LoadForApp();

    // `userSettingsFile` is out of scope. There is no risk to use it instead of
    // `appSettingsFile`.
}

Does it look wrong? Would you avoid such syntax? Is it difficult to understand by beginners?

share|improve this question
3  
Could you argue that each independent "scope block" should instead be its own method or function? –  Dan Pichelman Jun 6 '13 at 19:58
    
@ThomasEding: thank you. Corrected. –  MainMa Jun 7 '13 at 4:43
    
I would say "often" not as well implemented as it could be (but probably not a "design" problem). Sometimes by design, it is unavoidable (for instance exception/resource scoping). –  JustinC Jun 7 '13 at 7:47
add comment

3 Answers

up vote 21 down vote accepted

If your function is so long that you cannot recognize any unwanted side effects or illegal reuse of variables any more, then it is time to split it up in smaller functions - which makes an internal scope pointless.

To back this up by some personal experience: some years ago I inherited a C++ legacy project with ~150K lines of code, and it contained a few methods using exactly this technique. And guess what - any of those methods was definitely too long. In between, we refactored most of that code, the methods became smaller and smaller, and I am pretty sure there are no remaining "internal scope" methods any more - they are simply not needed any more.

share|improve this answer
    
But why is it bad? Having many named functions is also confusing. –  Kris Van Bael Jun 7 '13 at 5:22
1  
+1 Exactly, if you need a scope, likely you're performing a particular action that can be extracted to a function. Kris, it's not about creating lots and lots of functions, it's that when these cases occur (where one block's scope might conflict with another's) that usually a function should have been used. I see this in other languages (namely JavaScript, with IIFE's instead of plain' old functions) all the time too. –  Benjamin Gruenbaum Jun 7 '13 at 5:24
7  
@KrisVanBael: "Having many named functions is also confusing" - if you use descriptive names, quite the opposite is true. Creating too large functions is the topmost reason how code becomes hard to maintain, and even harder to extent. –  Doc Brown Jun 7 '13 at 5:44
1  
If there are so many functions that naming them distinctly becomes a problem, possibly some of them can be rolled up into a new type as they may have significant independence from their original function body. Some might be so closely related that they could be eliminated. Suddenly, there aren't quite so many functions. –  JustinC Jun 7 '13 at 7:44
    
Still not convinced. Often long functions are bad indeed. But saying that every need for scope requires a separate function is too black&white for me. –  Kris Van Bael Jun 8 '13 at 13:29
show 6 more comments

It is far easier for a beginner (and any programmer) to think of:

  • not using a variable that is not relevant

than to think of:

  • adding code structures aiming at preventing himself from not using a variable that is not relevant.

Moreover, from a reader's point of view, such blocks have no apparent role: removing them has no impact on the execution. The reader may scratch his head trying to guess what the coder wanted to achieve.

share|improve this answer
add comment

I agree that reusing variable is more often than not a code smell. Probably such code should be refactored in smaller, self-contained, blocks.

There is one particular scenario, OTOH, in C#, when they tend to popup - that is when using switch-case constructs. Situation is very nicely explained in:

http://stackoverflow.com/questions/3652408/c-sharp-switch-statement-with-without-curly-brackets-whats-the-difference

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.