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

Question

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?

Explanation / Answer

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.

Related Questions

Navigate

• Browse (All)
• Browse T
• Subjects

Integrity-first tutoring: explanations and feedback only — we do not complete graded work. Learn more.