How to avoid repeating variable initialization?

Question

I am writing in C#, but this question may apply to other languages as well.

public class Test
{
    int a = 10; // I created 'a' here
    public void M()
    {
        int a = 20; // I forgot that I already have 'a' in the class and I initial it again ;

        // do other stuff with "a"... 
    }
}

This way isn't against the declaring space rule, so the compiler will have no problem with it. I know I could use this.a if I want to access the a outside M() and they are not a variable actually.

My question is:

Is this way could make some people confused by allowing declare the same name in a sub scope? Will it be harder to debug or doing code review?

Answer 1

One can avoid these types of scenarios by using a tool like Resharper or other type of "clean code" / "linter" tool.

Most of these tools spot errors as soon as you type them in and will underline the offending lines of code and offer suggestions on to how to remedy.

Most projects I have been on, automatically run the linter/clean code tool prior to check in so developers are not allowed to check in code like the example cited above.

If you don't use a tool, then it's the responsibility of the developer to understand variable scope and not code in this fashion.

Personally, a good "clean code" / "linter" tool is a must for any non-trivial project as it will promote correct and consistent coding practices and allows one to spend more brain power solving the problem at hand. Also, it helps when refactoring and or modifying because one might be in a large class when someone else had already declared that variable and you are unaware of it.

Answer 2

Such name shadowing can also be useful at times. For example you cite C# and for that language the design team are considering relaxing the name shadowing rules for local functions (scroll down to the "Static local functions" section and read the third question and answer for reference).

However, it remains the case that for many situations variable/field shadowing can indeed cause confusion and can make the code harder to read and debug. To that end, there are a number of techniques you can employ to minimise it:

1. Use longer, meaningful names. Sure a is just an example in your code, but folk that regularly use short, often abbreviated names run into this problem more often. If your field and variable were something like someSortOfIntField and aCompletelyDifferentInt, then collisions are less likely.
2. Use a naming convention to minimise name collisions. For example in C#, prefixing fields with _ is a common convention. _a and a are clearly visibly different and so reduce the risk of confusion.
3. Use this to reference fields. I personally hate this approach, but others really like it. And code checking/linter tools are available that can enforce such conventions.
4. Use tooling that detects such shadowing and reports it as a warning or error. For example, the popular C# tool, ReSharper, will warn on such name shadowing by default.

Answer 3

As a general rule, we should declare variables in the innermost scope that is applicable, i.e. do not use a wider scope than necessary.

If a is being declared as an instance variable yet used as a local variable by the methods — i.e. always re-initializing it within the methods (not looking at the previous method's value) — then the instance variable is not maintaining (object) state between method invocations, and such a variable is being declared in a scope wider than necessary.  Under these circumstances, I would find the member variable at issue rather than the local variable that shadows it.

Another general rule that instance members should have the same lifetime as the object itself — if we find instance members that only come into play after certain state or conditions, that suggests we have another object (e.g. two or more objects are being conflated).

On the other hand, if the a instance variable is indeed carrying object state (i.e. is is used across/in-between methods invocations), then when a method (that because it is using a shadowing local variable of the same name) fails to use and/or update the instance variable, your tests ought to notice that something is amiss.  If they don't either insufficient tests or the variable wasn't actually holding object state.