Is it a waste of time to call a function in the foreach definition?

Question

Sometimes I have to run a loop where the list or array comes from a function.

I usually do it this way :

Dim list = SomeClass.GetMyList()
For Each item in list
  'Do some stuff
Next

Is it the same as :

For Each item in SomeClass.GetMyList()
  'Do some stuff
Next

I usually do the first way because I think that second way makes a call everytime it starts the next iteration, therefore wasting some time.

Am I right to think that way ? Or can I go ahead with second way as the compiler is smart enough not to make a call every round ?

In VB only once, in C# every time. Just print out something in that function, you'll see it will be called once. * Sorry, I made the mistake between FOR and FOR EACH, I'm not sure about the behavior of for each in C# — the_lotus, Jul 26 '16 at 13:19
The two methods are equivalent. You can easily test this by using a breakpoint or debug output from your `SomeClass.GetMyList()`. — Glorin Oakenfoot, Jul 26 '16 at 13:19
@the_lotus - C# will also only do it once. See, for instance *[How does foreach work when looping through function results?](http://stackoverflow.com/q/1632810/1364007)* — Wai Ha Lee, Jul 26 '16 at 13:20
In any case, it makes little sense to be discussing anything about C# given that this is a VB question. ;) — Glorin Oakenfoot, Jul 26 '16 at 13:22
Although it works, I personally find it way easier to read when looking at someone else's code when done in way No. 1. — Tom K., Jul 26 '16 at 13:24
Agree: "whatever you decide to do, make it *abundantly clear."* Speed or Efficiency is probably an irrelevant consideration. — Mike Robinson, Jul 26 '16 at 13:31
What kind of Type is the result of "GetMyList()" -array? list? whatever.. — Cadburry, Jul 26 '16 at 13:31
A now-removed Answer referred to the possibility of "iterators." If the function *is* an iterator, be sure to call it in just the way that the documentation prescribes. — Mike Robinson, Jul 26 '16 at 13:32
@MikeRobinson I thought an iterator would make a difference here but after posting I tried it out and it appears that the compiler is smart enough to only iterate over what is needed is both of the scenarios above so I deleted it — Matt Wilko, Jul 26 '16 at 14:05
In some cases, there might be some differences. See [Does foreach automatically call Dispose?](http://stackoverflow.com/questions/4982396/does-foreach-automatically-call-dispose). I suppose same thing apply for VB.NET. — Phil1970, Jul 26 '16 at 15:35

score 3 · Answer 1 · answered Jul 26 '16 at 13:35

Only what is within the for block gets repeated, not its initialiser.

Your 2nd option does the same as the 1st, just with an unnamed temporary variable holding the result of GetMyList(). If anything, it might be more efficient for that reason... though a good optimiser would make both pieces of code equivalent anyway.

As mentioned in the comments, a debugger would've made this abundantly clear, and is an invaluable tool for countless other reasons.

score 2 · Accepted Answer · edited Jun 20 '20 at 09:12

Method #1 leaves you with a reference to the list in the scope of the rest of the method.

Method #2 creates a variable behind the scenes referencing the list, but that variable is out of scope after the for loop

For scoping I would prefer #2, but I am also impartial to succinct code. If GetMyList returns a reference type such as a List<T> or array, this could leave the door open to some unintended side effects.

Public Sub Foo()
    Dim someClass As New SomeClass()
    ' this variable stays in scope after the following For Each loop
    Dim list = someClass.GetMyList()
    For Each item In list
        Console.Write(item)
    Next
    Console.WriteLine()
    ' now we can sort the backing field - did you intend for this to happen?
    list.Sort()
    ' the following For Each loop doesn't leave any reference behind
    For Each item In someClass.GetMyList()
        Console.Write(item)
    Next
End Sub

Private Class SomeClass
    Private _list As List(Of Integer) = {3, 2, 1}.ToList()
    Public Function GetMyList() As List(Of Integer)
        Return _list
    End Function
End Class

Foo() writes:

321

123

So you can actually manipulate the backing field after you were supposedly done with it!

Yep, it's always good to use the minimal possible scope, and nice example of the potential pitfalls. +1 — underscore_d, Jul 26 '16 at 15:55
Thank you for pointing that out. I'm aware of the debugger but was wondering about side effects also. This is one of them. — Martin Verjans, Jul 26 '16 at 16:58

Fredou · Answer 3 · 2016-07-26T14:12:32.343

let take a simple example

there is not much information, i will assume GetMyList is a list(of integer)

Module Module1
    Sub Main()

        test1()
        test2()

        Console.ReadKey(False)
    End Sub

    Sub test1()
        Dim list = SomeClass.GetMyList()
        For Each item In list
            Console.WriteLine(item)
        Next
    End Sub

    Sub test2()
        For Each item In SomeClass.GetMyList()
            Console.WriteLine(item)
        Next
    End Sub
End Module

Class SomeClass
    Public Shared Function GetMyList() As List(Of Integer)
        Dim aList = New List(Of Integer)

        aList.Add(1)
        aList.Add(2)
        aList.Add(3)

        Console.WriteLine("I am in the function now")

        Return aList
    End Function
End Class

you can run it yourself to see the behavior

now let look at the actual IL (compiled with debug)

test1 code;

.method public static 
    void test1 () cil managed 
{
    // Method begins at RVA 0x2120
    // Code size 64 (0x40)
    .maxstack 1
    .locals init (
        [0] class [mscorlib]System.Collections.Generic.List`1<int32> list,
        [1] int32 item,
        [2] valuetype [mscorlib]System.Collections.Generic.List`1/Enumerator<int32> VB$t_struct$L0,
        [3] bool VB$CG$t_bool$S0
    )

    IL_0000: nop
    IL_0001: call class [mscorlib]System.Collections.Generic.List`1<int32> ConsoleApplication1.SomeClass::GetMyList()
    IL_0006: stloc.0
    IL_0007: nop
    .try
    {
        IL_0008: ldloc.0
        IL_0009: callvirt instance valuetype [mscorlib]System.Collections.Generic.List`1/Enumerator<!0> class [mscorlib]System.Collections.Generic.List`1<int32>::GetEnumerator()
        IL_000e: stloc.2
        IL_000f: br.s IL_0021
        // loop start (head: IL_0021)
            IL_0011: ldloca.s VB$t_struct$L0
            IL_0013: call instance !0 valuetype [mscorlib]System.Collections.Generic.List`1/Enumerator<int32>::get_Current()
            IL_0018: stloc.1
            IL_0019: ldloc.1
            IL_001a: call void [mscorlib]System.Console::WriteLine(int32)
            IL_001f: nop
            IL_0020: nop

            IL_0021: ldloca.s VB$t_struct$L0
            IL_0023: call instance bool valuetype [mscorlib]System.Collections.Generic.List`1/Enumerator<int32>::MoveNext()
            IL_0028: stloc.3
            IL_0029: ldloc.3
            IL_002a: brtrue.s IL_0011
        // end loop

        IL_002c: nop
        IL_002d: leave.s IL_003e
    } // end .try
    finally
    {
        IL_002f: ldloca.s VB$t_struct$L0
        IL_0031: constrained. valuetype [mscorlib]System.Collections.Generic.List`1/Enumerator<int32>
        IL_0037: callvirt instance void [mscorlib]System.IDisposable::Dispose()
        IL_003c: nop
        IL_003d: endfinally
    } // end handler

    IL_003e: nop
    IL_003f: ret
} // end of method Module1::test1

test2 code;

.method public static 
    void test2 () cil managed 
{
    // Method begins at RVA 0x217c
    // Code size 62 (0x3e)
    .maxstack 1
    .locals init (
        [0] int32 item,
        [1] valuetype [mscorlib]System.Collections.Generic.List`1/Enumerator<int32> VB$t_struct$L0,
        [2] bool VB$CG$t_bool$S0
    )

    IL_0000: nop
    IL_0001: nop
    .try
    {
        IL_0002: call class [mscorlib]System.Collections.Generic.List`1<int32> ConsoleApplication1.SomeClass::GetMyList()
        IL_0007: callvirt instance valuetype [mscorlib]System.Collections.Generic.List`1/Enumerator<!0> class [mscorlib]System.Collections.Generic.List`1<int32>::GetEnumerator()
        IL_000c: stloc.1
        IL_000d: br.s IL_001f
        // loop start (head: IL_001f)
            IL_000f: ldloca.s VB$t_struct$L0
            IL_0011: call instance !0 valuetype [mscorlib]System.Collections.Generic.List`1/Enumerator<int32>::get_Current()
            IL_0016: stloc.0
            IL_0017: ldloc.0
            IL_0018: call void [mscorlib]System.Console::WriteLine(int32)
            IL_001d: nop
            IL_001e: nop

            IL_001f: ldloca.s VB$t_struct$L0
            IL_0021: call instance bool valuetype [mscorlib]System.Collections.Generic.List`1/Enumerator<int32>::MoveNext()
            IL_0026: stloc.2
            IL_0027: ldloc.2
            IL_0028: brtrue.s IL_000f
        // end loop

        IL_002a: nop
        IL_002b: leave.s IL_003c
    } // end .try
    finally
    {
        IL_002d: ldloca.s VB$t_struct$L0
        IL_002f: constrained. valuetype [mscorlib]System.Collections.Generic.List`1/Enumerator<int32>
        IL_0035: callvirt instance void [mscorlib]System.IDisposable::Dispose()
        IL_003a: nop
        IL_003b: endfinally
    } // end handler

    IL_003c: nop
    IL_003d: ret
} // end of method Module1::test2

only thing different is when the GetMyList reference is called / loaded in memory

first one load it in a local variable, the second one just load it when the loop start

so both scenario will do the same thing.

Is it a waste of time to call a function in the foreach definition?

3 Answers3