3

For a VBA program that uses a single object variable multiple times, is it best to reset the variable in the loop without first setting the variable to nothing (which can be done at the end outside of the loop), or should the variable always be set to nothing after each instance of use. From a reference counting perspective it seems best to set to nothing as I never have need for the previous state of the object once it has been changed to the next variable (I'm not sure how one would retrieve the previous state but I gather that there must be a function of such from this question). Another relevant question that I examined can be found here.

The application which triggered this question is one where I cycle through many Word documents, pulling relevant information from each of them. See example below:

dim objDoc as object, objWord as object
Set objWord = CreateObject("Word.Application")
for r=2 to 40
    'Some code to set filename 
    Set objDoc = objWord.Documents.Open(filename, ReadOnly:=False, Visible:=False)
    'Code to retrieve relevant info from document
    objDoc.close
    Set objDoc = nothing 'Should this go here?
next r
set objDoc = nothing 'Or should it go here?
objWord.Quit

The Question: Does it matter when you set an object variable to nothing in terms of code efficiency or problem avoidance. I have highlighted above two options for where I believe that should go with the third option being that it doesn't matter when I set the variable to nothing

Community
  • 1
  • 1
Ian
  • 187
  • 1
  • 10
  • 1
    For an unchanging object (or variable), For clarity sake I will set those outside of any loop, so that the loop concentrates on what's changing. Also, you don't have to set an object to `nothing` before performing another `set` for the same object variable. In your example, `set objDoc = nothing` should go outside the loop only. – PeterT May 04 '18 at 18:30
  • Mmm. In the very earlier VBA days (Office 97) sometimes it *did* matter. But the behavior was changed (fixed) fairly early on. Now it only matters if the objects come from an outside application, then it *could* lead to problems. In the particular case you show us, this is one of those things that could lead to problems, especially if the document doesn't complete closing before the loop runs again (race condition). In this case I would explicitly set to Nothing inside the loop to avoid any chance of conflict. – Cindy Meister May 04 '18 at 18:41
  • @CindyMeister I would actually argue that it **belongs** inside the loop body - be it only because if you refactor that code and extract a method out of the loop body, `objDoc` only needs to exist in the extracted method, and should therefore be created (and thus destroyed) inside that scope. – Mathieu Guindon May 04 '18 at 18:50

3 Answers3

3

I would argue that, generally speaking, having a variable that has two or more different meanings depending on context, is making the maintainer's life harder than it needs to be. In this case, whether you put it inside or outside the loop scope should make no difference at all - in fact whether Set objDoc = Nothing is specified at all, should make no difference either.

You're not showing the rest of the procedure, but as far as I can see there's not really a reason why that particular procedure wouldn't be doing 10,000 other things.

The code you've shown should be written in a bunch of procedures and functions.

'Some code to set filename 

You need a function that implements the logic to get a filename, and returns a String.

'Code to retrieve relevant info from document

You need a procedure that retrieves relevant info from a document.

Pull the body of the loop into its own procedure:

For r = 2 To 40
    filename = LogicToGetTheFilename(r)
    ProcessWordDocument wordApp, filename
Next

Much higher abstraction level here - with proper naming you know what that loop does at a glance.

Private Sub ProcessWordDocument(ByVal wordApp As Object, ByVal filename As String)
    Dim docx As Object 'Word.Document
    Set docx = wordApp.Documents.Open(filename, ...)
    'do stuff
    docx.Close
    'Set docx = Nothing
End Sub

Now that the Word document object variable is living in its own scope, VBA should appropriately destroy the underlying object whenever execution exits that scope (i.e. once per loop iteration).

Because refactoring the loop into proper abstraction levels means the Word document only lives for a single iteration, I would argue (rather strongly) that Set objDoc = Nothing belongs inside the loop body.

Mathieu Guindon
  • 69,817
  • 8
  • 107
  • 235
3

Short answer: In the loop.

Long answer: From a problem avoidance perspective, you should set it to 'nothing' when you are immediately done with it. The problem you are going to be trying to catch is an unintended reuse. You want your code to explode if you ever do accidentally reuse the variable instead of having to debug it later when it gives weird results. Generally speaking you shouldn't reuse variables because it's all too easy to make a mistake when refactoring and the variable ends up containing the wrong value.

VBA unfortunately doesn't care if you use a variable outside of the innerscope it was dim'ed in. The following is unfortunately legal and more modern languages don't let you do this:

Sub CountInsideLoop()

    Dim i As Long
    For i = 1 To 3
        Dim count As Long
        count = count + 1
    Next i

    ' count value will be 3
    Debug.Print count
End Sub
Not Saying
  • 194
  • 11
  • 1
    That's because the smallest scope is *procedure scope* in VBA. Hence I'd recommend pulling out into its own procedure, for proper scoping. No procedure should do 20 things anyway. +1 nicely worded ;-) – Mathieu Guindon May 04 '18 at 19:07
  • Thanks! I agree pulling it out into its own procedure is The Right Thing To Do™. Makes it sooo much easier to read, test and reuse. – Not Saying May 04 '18 at 19:32
0

Inside the loop

Which should be easily inferred from the following equivalent code:

dim objWord as object
Set objWord = CreateObject("Word.Application")

for r=2 to 40
    'Some code to set filename 
    With objWord.Documents.Open(filename, ReadOnly:=False, Visible:=False) 'Set and reference your document object
        'Code to retrieve relevant info from document
        .Close
    End With 'this “disposes” the referenced document object, so you also set it to Nothing
Next r
objWord.Quit

Of course, there’s is room for further application of the same concept to objWord, too:

With CreateObject("Word.Application") ‘ instantiate and reference your Word object
    for r=2 to 40
        'Some code to set filename 
        With .Documents.Open(filename, ReadOnly:=False, Visible:=False) 'Set and reference your document object
            'Code to retrieve relevant info from document
            .Close
        End With 'this “disposes” the referenced document object, so you also set it to Nothing
    Next r
    .Quit
End With 'this “disposes” the referenced Word object, so you also set it to Nothing
DisplayName
  • 13,283
  • 2
  • 11
  • 19