CppCheck. The scope of the variable can be reduced (and loop)

Question

CppCheck finds me some findings like: "The scope of the variable 'x' can be reduced".

What if I have this situation:

int x;
for (int i = 0; i != 10; ++i)
{
    x = someFunction();

    // ... I use x variable here
}

I think my code is OK. What do you think? Should it change to something like that?

for (int i = 0; i != 10; ++i)
{
    int x = someFunction();

    // ... I use x variable here
}

In the second code a variable x is defined for all iteration... Isn't not ok (not optimal), I guess..

Solution 1

The position of an int declaration has no performance impact, so Cppcheck is right when raising this style issue. This style issue can be applied to non-trivial types as well,

for (int i = 0; i != 10; ++i)
{
    MyType x = someFunction();

    // ... I use x variable here
}    

since constructors tend to be as equally efficient as assignments. As of Version 1.65, Cppcheck seems not to distinguish between trivial and non-trivial types.

But don't blindly follow such style suggestions, there will be cases of non-trivial types where assignment is more efficient than construction. (As usual: if in doubt about performance, measure!)

Edit: a style consideration

The second variant is better in style as it combines declaration and initialization:

• This often saves you from writing (or reading) a comment that is not very meaningful.
• Sometimes you can also add const, which prevents you from accidental changes

Solution 2

If variable x is not used outside the loop then the second approach is much better. And there is not the slightest problem with the optimization of the code. The memory for the variable is allocated only once in the loop.

Solution 3

As others have mentioned, for trivial types it is unlikely to make significant performance impact.

However, you should also consider that, by reducing scope, you aid readability by having the declaration closer to usage, and possibly more importantly, make it easier to refactor.

Both of these could be important when considering maintainability.

We all know we should keep functions short and well refactored, but we have all seen those 5000 line long monsters, where a variable was declared at the top, and used once, 3789 lines in. And if you haven't, pity the rest of us.

Comments

[Richard Hodges]

The allocation of stack memory for the variable will happen at the beginning of the function, not in the loop. The initialisation of x will take place in the loop. But I'm splitting hairs.

[peter55555]

How about not primitive types, for example std::string? The same?

[Wolf]

@pawell55555 non-primitive types may cause a performance hit. I just tried a non-trivial class, and Cppcheck wrongly insists in the same style issue.

[Wolf]

@pawell55555 I added an answer that also covers this question.