5
\$\begingroup\$

I'm often opening, from Excel, all Word documents inside a folder.

And sometimes during the treatments, Word crash without throwing an error.

So I've added a bit of error handling to restart Word and reopen the doc I was working on.


I'm just wondering if I'm doing it properly, or if I should go further like :

  1. Test the error number
  2. If it's 462 : The remote server machine does not exist or is unavailable, go to RestartWord
  3. If it's something else, show the error message

It may also not be only Error 462, but I don't have any other examples for this.


My code :

Private Sub Update_Word_DocMs_In_Folder(ByVal FolderToScan As String, _
                                            ByVal PathTxtWithInfo As String)
Dim oW As Object, NewFile As String
'''Boolean to detect if it was the initial launch
Dim NormalWordLaunch As Boolean
NormalWordLaunch = True
Set oW = OpenOrReopen("Word.Application", NormalWordLaunch)


On Error GoTo RestartWord
    NewFile = Dir(FolderToScan & "*.docm")
    Do While NewFile <> vbNullString
'''ErrHdl: Jump back here after re-creating a Word's instance
ReOpenWordFile:
        Call OpenWordDocAndApplyTreatment(oW, FolderToScan & NewFile, PathTxtWithInfo)
        NewFile = Dir()
    Loop
    oW.Quit
On Error GoTo 0

Set oW = Nothing
Set oDoc = Nothing
Exit Sub
RestartWord:
    If Err.Number <> 462 Then
        MsgBox Err.Number & vbCrLf & Err.Description, vbCritical + vbOKOnly, "Error not handled"
        DoEvents
        Resume
    Else
        Set oW = OpenOrReopen("Word.Application", NormalWordLaunch)
        '''ErrHdl: If this isn't the initial launch go back to the loop
        If Not NormalWordLaunch Then Resume ReOpenWordFile
    End If
End Sub

Function to handle creating an app instance :

Function OpenOrReopen(ByVal ClassName As String, NormalLaunch As Boolean) As Object
    Set OpenOrReopen = Nothing
    On Error Resume Next
    '''Check if there is already an instance
    Set OpenOrReopen = VBA.GetObject(, ClassName)
    On Error GoTo 0

    If OpenOrReopen Is Nothing Then
        '''If there isn't any instance, create one
        Set OpenOrReopen = VBA.CreateObject(ClassName)
    Else
        '''If detected instance is visible, create a new one
        If OpenOrReopen.Visible Then Set OpenOrReopen = VBA.CreateObject(ClassName)
    End If
    '''Set instance to be hidden
    OpenOrReopen.Visible = False
    OpenOrReopen.Options.UpdateLinksAtOpen = False

    '''For ErrHdl: Change boolean the 1st time
    If NormalLaunch Then NormalLaunch = False
End Function

Code for treatment :

Sub OpenWordDocAndApplyTreatment(WordApp As Object, _
                                 ByVal FileFullName As String, _
                                 ByVal PathTxtWithInfo As String)
    Dim oDoc As Object
    Set oDoc = WordApp.Documents.Open(FileFullName)
    With oDoc
        Do While .ReadOnly
            DoEvents
        Loop
        '''Some treatment to update doc's properties
        WordApp.Run "Properties_Updating.Properties_Update", PathTxtWithInfo
        DoEvents
        .Save
        .Close
    End With 'oDoc
End Sub
\$\endgroup\$
3
  • \$\begingroup\$ Are you closing the word application at all? during the loop I mean. \$\endgroup\$ Commented Mar 9, 2017 at 22:31
  • \$\begingroup\$ @Raystafarian : Nope, I just close it once I'm done processing the documents. So it's just unexpected crashes that happens inside the loop. \$\endgroup\$
    – R3uK
    Commented Mar 10, 2017 at 7:47
  • 1
    \$\begingroup\$ I'd use a SELECT CASE rather than IF on your error handler - easier to group any errors then. \$\endgroup\$ Commented Jul 18, 2017 at 13:12

1 Answer 1

2
\$\begingroup\$

The name Update_Word_DocMs_In_Folder isn't following the PascalCase convention that's otherwise observed by your code; underscores should generally be avoided in procedure names in VB6/VBA (especially for Public members), because underscores have a very specific meaning in identifier names:

[Interface]_[Member]
[EventSource]_[EventName]

So Properties_Update is a problem, or can become one.

I find the indentation hard to follow, it doesn't seem consistent: some statements aren't indented, others are, but there's no apparent rhyme or reason to it.

The Call statement is obsolete and its use isn't consistent either; I'd remove it.

Call OpenWordDocAndApplyTreatment(oW, FolderToScan & NewFile, PathTxtWithInfo)

Becomes:

OpenWordDocAndApplyTreatment oW, FolderToScan & NewFile, PathTxtWithInfo

The RestartWord label is misleading: it's just an error handler subroutine, it doesn't necessarily "restart Word".

oDoc and oW have that useless and annoying Systems Hungarian prefix ("o" for "object", right?) and the identifiers are rather poorly named. I would've had wordApp instead of oW (funny just noticed that's quite exactly what another procedure is calling it), and oDoc is declared and assigned to Nothing, but it's never assigned an actual object reference, never passed ByRef to anything, and never referenced either: it's not used, and should be removed.

Not sure why CreateObject is being fully-qualified with the VBA standard library identifier - seems rather arbitrary. Why isn't MsgBox fully-qualified then?

I find this a bit dangerous:

MsgBox Err.Number & vbCrLf & Err.Description, vbCritical + vbOKOnly, "Error not handled"
DoEvents
Resume

If the error is unknown and unhandled, IMO the right thing to do is to gracefully halt execution, not enter a potentially infinite "try that again in an unknown error state" loop.

If Not NormalWordLaunch Then Resume ReOpenWordFile

I don't like this jumping into a loop body. If OpenWordDocAndApplyTreatment is what's throwing error 462, then OpenWordDocAndApplyTreatment is what should be handling error 462 - I'd make the procedure a Function that returns False when that error is thrown and the Word instance needs to be reset.

Honestly, that procedure-that-should-be-a-function wants its own error handling: it can blow up given an invalid path/filename, or Documents.Open fails for whatever reason, or there's no Properties_Updating module, or there's no Properties_Update macro, or .Save fails - so many things can go wrong in that scope, and all of it is just bubbling back up to the calling code's loop body - that's why you have all this jumping-around.

Handle these errors in OpenWordDocAndApplyTreatment, and the loop body and its error-handling logic can get much more straightforward.

OpenOrReopen has its side-effects backwards IMO. I would have returned a Boolean and altered an Optional ByRef outWordApp As Object = Nothing parameter. The ClassName parameter is misleading too: that's a registered ProgId, not a ClassName. Not sure why a ProgId would be needed though - it looks like you could use it to get any registered object, but then there's this:

OpenOrReopen.Visible = False
OpenOrReopen.Options.UpdateLinksAtOpen = False

Which, aside from treating the return value as if it were a declared local variable (a bad practice IMO), assumes things it shouldn't be assuming about the object's members - if it's meant to only work with a Word.Application, then don't take a ProgId parameter, hard-code it in (or make it a Const somewhere, but don't pretend it's a parameter).

\$\endgroup\$

Not the answer you're looking for? Browse other questions tagged or ask your own question.