NotOneShotInitialization

Summary

Declare local variables with one-shot initialization.

Default severity

Warning

Description

Do NOT:

• Declare a local variable and initialize it with a provisional value
• And then, depending on some condition, assign another value to that variable

For example, see the following code:

public const int MaxScore = 100;

public void Foo(int score)
{
    var s = score;
    if (s > MaxScore)
    {
        s = MaxScore;
    }
    ...
}

Walter E. Brown calls this the “let me change my mind” idiom in [1].

You should rewrite it with one-shot initialization as follows:

...
var s = (score > MaxScore) ? MaxScore : score;
...

or

...
var s = Math.Min(score, MaxScore);
...

Let's see the next example:

public void Bar(string colorName)
{
    var c = -1;
    switch (colorName)
    {
        case "black":
            c = 0;
            break;
        case "blue":
            c = 1;
            break;
        case "red":
            c = 2;
            break;
        ...
    }
    ...
}

We can rewrite it with the switch expression as follows:

...
var c = colorName switch
{
    "black" => 0,
    "blue" => 1,
    "red" => 2,
    ...
    _ => -1,
};
...

or with Dictionary:

private static readonly Dictionary<string, int> colorMap
    = new Dictionary<string, int>()
{
    ["black"] = 0,
    ["blue"] = 1,
    ["red"] = 2,
    ...
};

public void Bar(string colorName)
{
    var c = colorMap.GetValueOrDefault(colorName, -1);
    // If you cannot use the 'GetValueOrDefault' method, try the following
    // code instead:
    //
    // var c = colorMap.TryGetValue(colorName, out var found) ? found : -1;
    ...
}

In addition to the above example, you can also move the initial value to another method that returns a value such as var s = Method();. Or, if the separation with another method causes a lot of parameters, you can define the local function instead of the method;

Initializing multiple local variables

Consider the following example:

bool b = ...;

var x = 0;
var y = 1;
if (b) {
    x = 1;
    y = 3;
}

Since you must not repeat the branches depending on the same condition, you should not write the following code:

var x = b ? 1 : 0;
var y = b ? 3 : 1;

Instead, you can use a tuple as follows:

var (x, y) = b ? (1, 3) : (0, 1);

Initialization with side effects

Let's consider more practical code as follows:

public void Baz()
{
    var n = UserInput;
    if (!IsValid(n))
    {
        logger.Warn($"invalid input: {n}");
        n = DefaultValue;
    }
    else
    {
        logger.Info($"input: {n}");
    }
    ...
}

Run

Even if you rewrite the above example with one-shot initialization, you should not write the following code:

public void Baz()
{
    var raw = UserInput;
    var isValid = IsValid(raw);
    if (!isValid)
    {
        logger.Warn($"invalid input: {raw}");
    }
    else
    {
        logger.Info($"input: {raw}");
    }
    var n = isValid ? raw : DefaultValue;
    ...
}

The branch depending on isValid should be one time as well as the original code. You can do so with local functions as follows:

public void Baz()
{
    int ValidUserInput(int raw)
    {
        logger.Info($"input: {raw}");
        return raw;
    }

    int InvalidUserInput(int raw)
    {
        logger.Warn($"invalid input: {raw}");
        return DefaultValue;
    }

    var raw = UserInput;
    var supplier = IsValid(raw)
        ? (Func<int, int>)ValidUserInput
        : InvalidUserInput;
    var n = supplier(raw);
    ...
}

Run

Or more simply, you can also use a tuple containing the action of type Action that represents side effects, as follows:

public void Baz()
{
    var raw = UserInput;
    var (n, action) = IsValid(raw)
        ? (raw, new Action(() => logger.Info($"input: {raw}")))
        : (DefaultValue, () => logger.Warn($"invalid input: {raw}"));
    action();
    ...
}

Run

Note that this analyzer does not emit diagnostics against the code examples described in this section.

Code fix

The code fix is not provided.

Example

Diagnostic

var b = 1;
if (...) {
    b = 3;
}

var n = 0;
if (...) {
    ++n;
}

var v = 0;
switch (...)
{
    case ...:
        v = 1;
        break;
    case ...:
        v = 2;
        break;
}

References

[1] Walter E. Brown. Crazy Code, Crazy Coders, Meeting C++ 2019