2

I was hoping someone could have a look and tidy this up for me; I have to say error handling is not strong point of mine. I have the code block below and I have been playing around with some error handles but it is not as I really want it.

What I am trying to do is ensure that if at any point there is an error the workbook and excel instance I have opened are closed down gracefully.

I am sure there are much nicer and simpler ways to achieve this than what I have come up with.

Sub QOScode()

On Error GoTo Fail

Dim app As New Excel.Application
app.Visible = False 'Visible is False by default, so this isn't necessary
Dim book As Excel.Workbook
Set book = app.Workbooks.Add(ActiveWorkbook.Path & "\QOS DGL stuff.xlsx")
'set up error handeling so if any thing happens the instance of excel with QOS sheets is closed gracefully

On Error GoTo Closebook
' MsgBox book.Sheets("ACLS").Cells(3, 3)
'Do what you have to do
' 

Closebook:
On Error Resume Next
book.Close SaveChanges:=False
app.Quit
Set app = Nothing
On Error GoTo 0
Fail:
End Sub

What I want is a single On error - close app and exit sub.

Can anyone provide a sample of what would be considered best practice for doing this?

Cheers

Aaron

So this code below, when the sheet does not exist it will cause the error, why does it not skip the "book.close" statement, I know this throws an error, but I want it to ignore it?

Sub QOScode()

On Error GoTo Closebook

Dim app As New Excel.Application
app.Visible = False
Dim book As Excel.Workbook
Set book = app.Workbooks.Add(ActiveWorkbook.Path & "\QOaS DGL stuff.xlsx") 'this sheet does not exist
'
MsgBox book.Sheets("ACLS").Cells(3, 3)
'Do what you have to do
'

Closebook:
Err.Clear
On Error Resume Next
book.Close SaveChanges:=False  'Object variable or with block variable not set (error 91)
app.Quit
Set app = Nothing
On Error GoTo 0

End Sub
4
  • So, what exactly is wrong with your current code? What is it that you want that does not happen? Commented Feb 11, 2012 at 22:41
  • Well nothing :) Unless of course for some reason an error prematurely closed the workbook and then "on error GOTO Closebook" was called. Even with the "GOTO next" statement, it still exits on the book.close SaveChanges:=False, with an error the object dose not exist. Is there a way to check that "book" exists, so I could do "If book exists then book.close" Commented Feb 11, 2012 at 23:01
  • But On Error Goto Next will make it go to the next statement which is app.Quit at which point everything is as you want it to be... Commented Feb 11, 2012 at 23:15
  • Sorry I meant to type "on error resume Next" In my current code, if for any reason the app or workbook get closed unexpectlay (there is no instance of them). the code errors out at the line "book.close" I expected that with the "resume next" it would just skip the line. I think may be the err.clear might be what i need. Commented Feb 11, 2012 at 23:29

3 Answers 3

10

My 2 cents on Error Handling.

You should always do error handling.

Some of the Reasons

1) You wouldn't want your app to break down and leave your users hanging! Imagine the frustration that it would cause them.

2) Error handling doesn't mean that you are trying to ignore error.

3) Error handling is neither defensive programming or aggressive programming. IMHO it is proactive programming.

4) Very few people are aware that you can find out the line which is causing the error. The property that I am talking about is ERL. Consider this example

Sub Sample()
    Dim i As Long
    Dim j As Long, k As Long

10  On Error GoTo Whoa

20  i = 5
30  j = "Sid"
40  k = i * j

50  MsgBox k

60  Exit Sub
Whoa:
70  MsgBox "Description  : " & Err.Description & vbNewLine & _
    "Error Number : " & Err.Number & vbNewLine & _
    "Error at Line: " & Erl
End Sub

enter image description here

5) In subs like worksheet change event, it is a must to do error handling. Imagine you have set the Enable Event to False and your code breaks! The code won't run next time till you set the events back to true

6) I can go on and on :-) Would recommend this link

Topic: To ‘Err’ is Human

Link: http://www.siddharthrout.com/2011/08/01/to-err-is-human/

Tip:

Use MZ Tools. It is free!

Here is how I would write your code.

Sub QOScode()
    Dim app As New Excel.Application
    Dim book As Excel.Workbook

10  On Error GoTo Whoa

20  Set book = app.Workbooks.Open(ActiveWorkbook.Path & "\QOS DGL stuff.xlsx")

30  MsgBox book.Sheets("ACLS").Cells(3, 3)

    '
    'Do what you have to do
    '
LetsContinue:
40  On Error Resume Next
50  book.Close SaveChanges:=False
60  Set book = Nothing
70  app.Quit
80  Set app = Nothing
90  On Error GoTo 0
100 Exit Sub
Whoa:
110 MsgBox "Description  : " & Err.Description & vbNewLine & _
           "Error Number : " & Err.Number & vbNewLine & _
           "Error at Line: " & Erl
120 Resume LetsContinue
End Sub
Sign up to request clarification or add additional context in comments.

22 Comments

+1. MZ Tools is very useful when writing major code, especially for public release.
Yeah. One of the useful feature of MZ Tools which I frequently use is numbering my code lines. :)
+1 "Very few people are aware of Erl" Yeah, that could have something to do with it being completely undocumented as far as I can tell. Microsoft did a good job of hiding that one.
HI, I agree with what you say And your Whoa: block is the kind of think I would use (defiantly in production code). my issue is though that even in your code. Say line "30" causes the workbook to crash out and throws an error which calls "Whoa:". The code will still then error out at line "book.close" and the script will halt rather then resume the next line "app.quit"
@DevilWAH: No it will not halt :) Did you try the above code? I just tested it by creating a new workbook called QOS DGL stuff.xlsx. And the error happened at line 30 as there was no sheet called "ACLS". It gave the error message and exited gracefully :)
|
5

I am not quite sure I understand your objective. If I did, I would probably disagree.

Is this code you are developing? I almost never use error handling in code I am developing. I want the interpreter to stop on the statement that gives the error. I want to understand why that error has occurred. What could I have done to avoid the error? Did I fail to check the file exists? Did I fail to check the path is accessable? I will add the missing code before I do anything else.

By the time you have finished development, you should plan for there to be no error condition for which you have not included proper code. Of course, this is impossible; you cannot make your code foolproof because fools are so ingenious. The version you release to users must contains error handling.

But you could not release this code to users since it would stop without warning. Would the user guess something had gone wrong with the macro or would they assume this was what was supposed to happen? If they decide the macro has failed what are they going to say to you? "It did not do what I was expecting and I do not know why." What are you going to say back? "What were you doing?" I do not think I have ever had a user give a believable description of what they were doing at the time of a failure. At the very least you want:

Call MsgBox("Sorry I have had an unrecoverable error within QOScode()." & _
            " Please record: " & Err.Number & " " & Err.Description & _
            " and report to extension 1234")

With this, the user does not wonder if something has gone wrong and you know where it went wrong and, with luck, why.

1 Comment

I am calling a second app that is hidden, so while developing (an indeed in the final code)if any thing happens I want a routine that will gracefully close this down, Other wise every time I am running a debug and some thing unexpected happens, the code quits and I am left with the hidden app still running. So like you say, its important to cover all possibilities, having code that will gracefully shut down the app, means what ever other error code I have for the rest of the sub, I can be sure that this always get called, After error messages have been displayed, or logs written.
5

To handle the foreseen issue neatly you can use short error handling to test that the Workbook actually exists (ie If Not Wb Is Nothing Then, if so work on it, with a common ending (ie destroying the object)

The second sample shows how to add additional handling for unforeseen errors once the workbook has been opened.I have used Err.Raise to create deliberate error to give the user a choice as to how to proceed (close the workbook immediately post error or make the workbook visible)

As an aside, don't use Dim and New together. I have re-written
Dim app As New Excel.Application
into
Dim xlApp As Excel.Application
Set xlApp = New Excel.Application

1. Handling the no workbook issue

Sub QOScode()
    Dim xlApp As Excel.Application
    Set xlApp = New Excel.Application
    Dim Wb As Excel.Workbook
    On Error Resume Next
    Set Wb = xlApp.Workbooks.Add(ActiveWorkbook.Path & "\QOaS DGL stuff.xlsx")    'this sheet does not exist
    On Error GoTo 0    '

    If Not Wb Is Nothing Then
        MsgBox Wb.Sheets("ACLS").Cells(3, 3)
        'Do what you have to do
        Wb.Close False
    End If

    xlApp.Quit
    Set xlApp = Nothing
End Sub

2. Handling no workbook and other unforseen error which may otherwise leave the worbook open

Sub QOScode2()
    Dim xlApp As Excel.Application
    Set xlApp = New Excel.Application
    Dim Wb As Excel.Workbook
    Dim lngChk As Long
    On Error Resume Next
    Set Wb = xlApp.Workbooks.Add(ActiveWorkbook.Path & "\QOaS DGL stuff.xlsx")    'this sheet does not exist
    On Error GoTo 0    '

    If Not Wb Is Nothing Then
        On Error GoTo ProblemHandler
        MsgBox Wb.Sheets("ACLS").Cells(3, 3)
        'Do what you have to do

        'Deliberate error
        Err.Raise 2000, "test code", "Sample Error"
        Wb.Close False
    Else
        MsgBox "Workbook not found,code will now exit"
    End If

    xlApp.Quit
    Set xlApp = Nothing
    Exit Sub

ProblemHandler:
    'Test to see if workbook was still open when error happened
    If Not Wb Is Nothing Then
        lngChk = MsgBox("The code encountered an error" & vbNewLine & Err.Number & vbNewLine & "Do you want to close the file?", vbYesNo, Err.Description)
        If lngChk = vbYes Then
            'close book. Code will proceed to destroy app
            Wb.Close False
        Else
            'make workbook visible and leave code
            Wb.Visible = True
            Exit Sub
        End If
    Else
        MsgBox "The code encountered an error - the file was already closed at this point" & vbNewLine & "Error number " & Err.Number & vbNewLine, Err.Description
    End If
    'destroy app (either if the workbook was closed, or user chose to close it)
    xlApp.Quit
    Set xlApp = Nothing

End Sub

2 Comments

This would solve the issue I am having, but does any one know why the code I have errors out even with the "resume next statement? why does it not just ignore the "book.close False" line in my original code?
Cheers for the tips, I like the idea of checking if work book exists and then acting on it, this it what I was looking for as I think its much nice to handle foreseen errors with logic and leave error handling to the unforeseen stuff.

Your Answer

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.