Is it better coding practice to define variables outside a foreach even though more verbose?

Question

In the following examples:

• the first seems more verbose but less wasteful of resources
• the second is less verbose but more wasteful of resources (redefines string each loop)

Which is better coding practice?

First example:

using System;
using System.Collections.Generic;

namespace TestForeach23434
{
    class Program
    {
        static void Main(string[] args)
        {
            List<string> names = new List<string> { "one", "two", "two", "three", "four", "four" };

            string test1 = "";
            string test2 = "";
            string test3 = "";
            foreach (var name in names)
            {
                test1 = name + "1";
                test2 = name + "2";
                test3 = name + "3";
                Console.WriteLine("{0}, {1}, {2}", test1, test2, test3);
            }
            Console.ReadLine();
        }
    }
}

Second example:

using System;
using System.Collections.Generic;

namespace TestForeach23434
{
    class Program
    {
        static void Main(string[] args)
        {
            List<string> names = new List<string> { "one", "two", "two", "three", "four", "four" };

            foreach (var name in names)
            {
                string test1 = name + "1";
                string test2 = name + "2";
                string test3 = name + "3";
                Console.WriteLine("{0}, {1}, {2}", test1, test2, test3);
            }
            Console.ReadLine();
        }
    }
}

Answer 1

The second form is no more wasteful - it's simply better.

There's no advantage to declaring the variables outside the loop, unless you want to maintain their values between iterations.

(Note that usually this makes no behavioural difference, but that's not true if the variables are being captured by a lambda expression or anonymous method.)

Answer 2

Personally, I think it's the best practice to declare variables in the tightest scope possible, given their usage.

This provides many benefits:

1. It's easier for refactoring, since extracting a method is simpler when the variables are already in the same scope.
2. The variable usage is more clear, which will lead to more reliable code.

The only (potential) disadvantage would be the extra variable declaration - however, the JIT tends to optimize this issue away, so it's one I wouldn't necessarily worry about in real work.

The one exception to this:

If your variable is going to be adding a lot of GC pressure, and if this can be avoided by reusing the same object instance through the foreach/for loop, and if the GC pressure is causing measured performance problems, I'd lift it into the outer scope.

Answer 3

Those are both wasteful and verbose.

foreach (var name in names)
{
   Console.WriteLine("{0}1, {0}2, {0}3", name);
}

.

</tongueincheek>

Answer 4

Depending on language and compiler it may or may not be the same. For C# I expect the resulting code to be very similar.

My own philosophy on this is simple:

Optimize for ease of understanding.

Anything else is premature optimization! The biggest bottleneck in most development is time and attention of the developer. If you absolutely must squeeze out every last CPU cycle then by all means do so, but unless you have a compelling business need to do or are writing a critical component (common library, operating system kernel, etc) you are better off waiting until you can benchmark the finished program. At that time optimizing a few of the most costly routines is justified, before then it's almost certainly a waste of time.