Is this "move declaration closer to usage" really preferable?

Question

Possible Duplicate:
Is it better to declare a variable inside or outside a loop?

Resharper wants me to change this:

int Platypus;
string duckBill1;
string duckBill2;
string duckBill3;
. . .
using (OracleDataReader odr = ocmd.ExecuteReader()) {
    while (odr.Read()) {
        Platypus = odr.GetInt32("Platypus");
        duckBill1 = odr.GetString("duckBill1");
        duckBill2 = odr.GetString("duckBill2");
        duckBill3 = odr.GetString("duckBill3");
        switch (Platypus) {
        . . .

...to this:

using (OracleDataReader odr = ocmd.ExecuteReader()) {
    while (odr.Read()) {
        int Platypus = odr.GetInt32("Platypus");
        string duckBill1 = odr.GetString("duckBill1");
        string duckBill2 = odr.GetString("duckBill2");
        string duckBill3 = odr.GetString("duckBill3");
        switch (Platypus) {
        . . .

...but in this way (it seems, at least, that) the vars are being declared N times, once for each time through the while loop. Is the Resharperized way really better than the original?

score 17 · Accepted Answer · answered Aug 16 '12 at 16:45

17

Yes, it is better because you're limiting the scope of the declared variables. There will be no performance impact for declaring them inside the loop. The reason Resharper is suggesting this change is that you're not using them outside of the loop.

answered Aug 16 '12 at 16:45

rsbarro

27,021
9
71
75

5

And if you declare them outside the loop, their scope will be broader, which means that it will take longer for them to be released. +1 – Andre Calil Aug 16 '12 at 16:46
2

@AndreCalil No it won't, because scope has nothing to do with collection eligibility. The performance impact is nil, as rsbarro says. – Jon Hanna Aug 16 '12 at 16:50
@JonHanna I'm sorry, I don't see where collections are in the subject. If you declare a variable on the scope of a method, it won't be released until the end of the method. If the scope is just a loop (like that `while`), it will be released sooner. And even a null string allocates memory: http://stackoverflow.com/a/6601485/1484750 – Andre Calil Aug 16 '12 at 16:54
1

@AndreCalil, GC is done not based on scope of variable, but on the fact if a value of the variable can be reached from GC roots at any particular point. As soon as variable is **verifiablly** no longer used in a method the value will be eligible for GC (http://stackoverflow.com/questions/7165353/does-garbage-collection-run-during-debug), there are good articles on MSDN and blogs.msnd.com which I can't find right now. – Alexei Levenkov Aug 16 '12 at 17:01
@AlexeiLevenkov Ok, but besides GC we are talking about a variable space reserved on the stack of the process. GC may free resources from the heap, but it won't modify the stack. – Andre Calil Aug 16 '12 at 17:03
1

@AndreCalil, to my knowledge the stack allocation happens exactly the same irrespective where you define the variables. I'll be glad to learn otherwise if you have links... – Alexei Levenkov Aug 16 '12 at 17:08
@AndreCalil That's the other side of Alexei's point though - GC happens when there's no live reference on the stack, and stack gets re-used when there's a use for it and the local in question won't be needed again. Scope has nothing to do with it, because scope is about C# rules about where a coder *can* use a variable, while stack-space is about where a coder *did* use a variable. It's not like the compiler has to worry about the coder adding another use at the end of the method - if they do it'll just compile the method differently the next time. – Jon Hanna Aug 16 '12 at 17:09
@AlexeiLevenkov http://stackoverflow.com/a/3943967/400547 I remember a better one and scarily enough trying to find it on google I found this comment-thread! There is one case where Andre is correct, which is if you create an object in starting a `using` block, but that's because there's an implied use of the object at the end of the scope, and it's that rather than the scope which keeps the reference on the stack. – Jon Hanna Aug 16 '12 at 17:25

Nicholas Carey · Answer 2 · 2014-10-30T16:44:59.983

Generally speaking, it is good programming practice to declare variables in as narrow a scope as possible. The reasons being:

Information Hiding.
Easier to understand.
Less likelyhood of bolluxing something up.

Even though it seems as if the variables are newly declared with each iteration over the loop, they are declared at compile time, not run time. Space is allocated on the stack frame for the variable[s], and that same space is reused for each iteration over the loop.

score 4 · Answer 3 · answered Aug 16 '12 at 16:49

4

Yes, but declaring them doesn't take any time at runtime. They don't take up any more memory becaues the compiler will just reuse their memory locations.

answered Aug 16 '12 at 16:49

Daniel

6,595
9
38
70

score 2 · Answer 4 · answered Aug 16 '12 at 16:48

2

The compiler will generally optimize such expressions and "lift" the variable declaration outside of the loop, since the variable itself is not dependent on the loop conditions. This effectively produces the code you demonstrated in the first example.

In this case, Resharper's suggestion is just to remove some redundant lines of code, in addition to reducing their pre-compiled scope.

answered Aug 16 '12 at 16:48

Steve Guidi

19,700
9
74
90

There isn't even anything to lift, the declaration just says "when this name is used within this scope, it means...", which doesn't translate to the compiled code. However, if it assigned with the declaration, and that assignment wasn't over-written later in the loop (which would change meaning anyway) then that would indeed most likely be lifted. – Jon Hanna Aug 16 '12 at 16:57

Is this "move declaration closer to usage" really preferable?

4 Answers4