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
200_success
145k22 gold badges190 silver badges478 bronze badges
asked Mar 9, 2017 at 15:06
\$\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\$ 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).

answered Dec 14, 2017 at 16:35
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.