[Okta Webinar] Learn how to a build a cloud-first strategyRegister Now

x
  • Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 190
  • Last Modified:

Error handler fails to reset itself after the second error

I'm not sure if I phrased the question right but please take a look at this code. As long as there is a file with name strXLS or strXLSM, the macro works fine. But if the macro doesn't find either one of those it fails to go to the third error handler and instead tries to open strXLSM. How do I fix that?

This code is based on an answer to a previous question and you might find some insight into what I'm doing wrong there. http://www.experts-exchange.com/Software/Office_Productivity/Office_Suites/MS_Office/Excel/Q_27325483.html

Thanks,
John
Dim cel As Range
For Each cel In ActiveSheet.[Customers]
    Dim wb As String, wb2 As String, strXLS As String, strXLSM As String
    wb = cel & " " & ActiveSheet.[annum] & " " & "Week " & [F1] & "_2bCoded.xls"
    wb2 = cel & " " & ActiveSheet.[annum] & " " & "Week " & [F1] & "_2bCoded.xlsm"
    strXLS = Left(ThisWorkbook.Path, Len(ThisWorkbook.Path) - 11) & cel & "\3_Working Files\2BCoded\" & wb
    strXLSM = Left(ThisWorkbook.Path, Len(ThisWorkbook.Path) - 11) & cel & "\3_Working Files\2BCoded\" & wb2
    Application.EnableEvents = False
    Application.DisplayAlerts = False
On Error GoTo London 'if first file doesn't open
    Workbooks.Open Filename:=strXLS
        On Error GoTo 0 'reset On Error when what you're looking to trap is no longer valid
        GoTo Paris
London:
On Error GoTo Rome 'if second file doesn't open
    Workbooks.Open Filename:=strXLSM
        On Error GoTo 0 'reset On Error when what you're looking to trap is no longer valid
Paris:
    Call AddNewWeek
Rome:
        On Error GoTo 0 'reset On Error when what you're looking to trap is no longer valid
Next cel

Open in new window

0
gabrielPennyback
Asked:
gabrielPennyback
  • 5
  • 2
1 Solution
 
dlmilleCommented:
Apologies, John.  Apparently, we're proceeding into the next error block without having fully resolving the original error:

According to Chip:  http://www.cpearson.com/excel/errorhandling.htm

in the case of:

    On Error GoTo Err1:
    Debug.Print 1 / 0
    ' more code
Err1:
    On Error GoTo Err2:
    Debug.Print 1 / 0
    ' more code
Err2:

- When the first error is raised, execution transfers to the line following Err1:. The error hander is still active when the second error occurs, and therefore the second error is not trapped by the On Error statement.


So, we need to rewrite the code a bit - at least to resolve the error causing the call to London.  The most expeditious way (you know my preferred- see the related question):
 
Dim cel As Range

    Application.EnableEvents = False
    Application.DisplayAlerts = False
    
    For Each cel In ActiveSheet.[Customers]
        Dim wb As String, wb2 As String, strXLS As String, strXLSM As String
        wb = cel & " " & ActiveSheet.[annum] & " " & "Week " & [F1] & "_2bCoded.xls"
        wb2 = cel & " " & ActiveSheet.[annum] & " " & "Week " & [F1] & "_2bCoded.xlsm"
        strXLS = Left(ThisWorkbook.Path, Len(ThisWorkbook.Path) - 11) & cel & "\3_Working Files\2BCoded\" & wb
        strXLSM = Left(ThisWorkbook.Path, Len(ThisWorkbook.Path) - 11) & cel & "\3_Working Files\2BCoded\" & wb2

    On Error GoTo London 'if first file doesn't open
        Workbooks.Open Filename:=strXLS
            On Error GoTo 0 'reset On Error when what you're looking to trap is no longer valid
            GoTo Paris
London:
    Resume LondonContinue 'resolve the error that got you to this stub
    
LondonContinue:
    On Error GoTo Rome 'if second file doesn't open

        Workbooks.Open Filename:=strXLSM
            On Error GoTo 0 'reset On Error when what you're looking to trap is no longer valid
Paris:
        Call AddNewWeek
Rome:
            On Error GoTo 0 'reset On Error when what you're looking to trap is no longer valid
    Next cel

    Application.EnableEvents = True
    Application.DisplayAlerts = True

Open in new window


PS - I moved your DisplayAlerts and EnableEvents commands out of the loop as it didn't appear to add value more than the first time they were executed.

Here's an alternative that might read a bit easier:
   
Dim cel As Range

    Application.EnableEvents = False
    Application.DisplayAlerts = False
    
    On Error Resume Next
    For Each cel In ActiveSheet.[Customers]
        Dim wb As String, wb2 As String, strXLS As String, strXLSM As String
        wb = cel & " " & ActiveSheet.[annum] & " " & "Week " & [F1] & "_2bCoded.xls"
        wb2 = cel & " " & ActiveSheet.[annum] & " " & "Week " & [F1] & "_2bCoded.xlsm"
        strXLS = Left(ThisWorkbook.Path, Len(ThisWorkbook.Path) - 11) & cel & "\3_Working Files\2BCoded\" & wb
        strXLSM = Left(ThisWorkbook.Path, Len(ThisWorkbook.Path) - 11) & cel & "\3_Working Files\2BCoded\" & wb2

        Workbooks.Open Filename:=strXLS
        If Err.Number <> 0 Then GoTo London
        
            On Error GoTo 0 'reset On Error when what you're looking to trap is no longer valid
            GoTo Paris
London:

        Workbooks.Open Filename:=strXLSM
        If Err.Number <> 0 Then GoTo Rome
            On Error GoTo 0 'reset On Error when what you're looking to trap is no longer valid
Paris:
        Call AddNewWeek
Rome:
            On Error GoTo 0 'reset On Error when what you're looking to trap is no longer valid
    Next cel

    Application.EnableEvents = True
    Application.DisplayAlerts = True

Open in new window


Dave
0
 
gabrielPennybackAuthor Commented:
Hi Dave, thanks. Your solutions look great, so I can't figure out why it still bugs. What's weird is that when I first ran it, it bugged on the fifth round, the first round that had neither of the two possibilities. I was a able to fix that by moving Application.DisplayAlerts = False inside the For loop. But then it bugged on the eighth round, the next round with no workbook matching either of the criteria. It bugs on this line:
Workbooks.Open Filename:=strXLS

Can you see anything that could be causing this failure of the final error handler, and how to fix it?

Thanks,
John
0
 
dlmilleCommented:
Can you download your current version for examination?  I tested the code I gave you (only through once, but may need more testing...) so let me see what the latest looks like...

Thanks,

Dave
0
Concerto Cloud for Software Providers & ISVs

Can Concerto Cloud Services help you focus on evolving your application offerings, while delivering the best cloud experience to your customers? From DevOps to revenue models and customer support, the answer is yes!

Learn how Concerto can help you.

 
dlmilleCommented:
I believe I erred again, with the reset (I generally use the err.number <> 0 approach to checking for errors), not quite following Chip's guidance.

I'm not sure how moving Application.Displayalerts into the loop helps anything - once its set to False, it stays that way (unless you call a routine that setting it back to true?  perhaps that's the case?)

let's try this - clearing the Rome error condition with the Resume RomeContinue.
Dim cel As Range

    Application.EnableEvents = False
    Application.DisplayAlerts = False
    
    For Each cel In ActiveSheet.[Customers]
        Dim wb As String, wb2 As String, strXLS As String, strXLSM As String
        wb = cel & " " & ActiveSheet.[annum] & " " & "Week " & [F1] & "_2bCoded.xls"
        wb2 = cel & " " & ActiveSheet.[annum] & " " & "Week " & [F1] & "_2bCoded.xlsm"
        strXLS = Left(ThisWorkbook.Path, Len(ThisWorkbook.Path) - 11) & cel & "\3_Working Files\2BCoded\" & wb
        strXLSM = Left(ThisWorkbook.Path, Len(ThisWorkbook.Path) - 11) & cel & "\3_Working Files\2BCoded\" & wb2

    On Error GoTo London 'if first file doesn't open
        Workbooks.Open Filename:=strXLS
            On Error GoTo 0 'reset On Error when what you're looking to trap is no longer valid
            GoTo Paris
London:
    Resume LondonContinue 'resolve the error that got you to this stub
    
LondonContinue:
    On Error GoTo Rome 'if second file doesn't open

        Workbooks.Open Filename:=strXLSM
            On Error GoTo 0 'reset On Error when what you're looking to trap is no longer valid
Paris:
        Call AddNewWeek
Rome:
    Resume RomeContinue 'resolve the error that got you to this stub
RomeContinue:
            On Error GoTo 0 'reset On Error when what you're looking to trap is no longer valid
    Next cel

    Application.EnableEvents = True
    Application.DisplayAlerts = True

Open in new window



PS - It perhaps is a matter of style, but I think the check for err.number <> 0 or function approach to handling errors (from prior question) is more my preference, as this approach "feels" more awkward to me (probably from lack of experience with cascading on error goto label: approaches) - so its a learning for me as well.

Hopefully, this "last" change will be the last ;)

Dave
0
 
dlmilleCommented:
We're going to need one minor modification - can't navigate to a Resume statement, without having hit an error first.  so, after Call AddNewWeek, we'll Goto RomeContinue:

 
Dim cel As Range

    Application.EnableEvents = False
    Application.DisplayAlerts = False
    
    For Each cel In ActiveSheet.[Customers]
        Dim wb As String, wb2 As String, strXLS As String, strXLSM As String
        wb = cel & " " & ActiveSheet.[annum] & " " & "Week " & [F1] & "_2bCoded.xls"
        wb2 = cel & " " & ActiveSheet.[annum] & " " & "Week " & [F1] & "_2bCoded.xlsm"
        strXLS = Left(ThisWorkbook.Path, Len(ThisWorkbook.Path) - 11) & cel & "\3_Working Files\2BCoded\" & wb
        strXLSM = Left(ThisWorkbook.Path, Len(ThisWorkbook.Path) - 11) & cel & "\3_Working Files\2BCoded\" & wb2

    On Error GoTo London 'if first file doesn't open
        Workbooks.Open Filename:=strXLS
            On Error GoTo 0 'reset On Error when what you're looking to trap is no longer valid
            GoTo Paris
London:
    Resume LondonContinue 'resolve the error that got you to this stub
    
LondonContinue:
    On Error GoTo Rome 'if second file doesn't open

        Workbooks.Open Filename:=strXLSM
            On Error GoTo 0 'reset On Error when what you're looking to trap is no longer valid
Paris:
        Call AddNewWeek
        GoTo RomeContinue
Rome:
    Resume RomeContinue 'resolve the error that got you to this stub
RomeContinue:
            On Error GoTo 0 'reset On Error when what you're looking to trap is no longer valid
    Next cel

    Application.EnableEvents = True
    Application.DisplayAlerts = True

Open in new window


Dave
0
 
Rory ArchibaldCommented:
>>"probably from lack of experience with cascading on error goto label: approaches"

And there's a good thing. All you get that way is spaghetti code that's horrendous to read and maintain. ;)
0
 
gabrielPennybackAuthor Commented:
Perfect, thanks Dave. So tell me if I have this logic right:

If "Workbooks.Open Filename:=strXLS" is successful, then we just go straight to Paris, pop the cork on some champagne and celebrate by calling "AddNewWeek'
If it's not successful, then in effect we go back to London and start all over again. If "Workbooks.Open Filename:=strXLSM" is successful, then I guess we go to the local pub and celebrate by calling "AddNewWeek'
If "Workbooks.Open Filename:=strXLSM" is also NOT successful, then we just say oh the heck with and go to Rome for a vacation.

I think I've got that right. :-)

- John

0
 
dlmilleCommented:
I can think of a few other things do do in Rome :)  Actually, I'd rather Greece.
0

Featured Post

Concerto Cloud for Software Providers & ISVs

Can Concerto Cloud Services help you focus on evolving your application offerings, while delivering the best cloud experience to your customers? From DevOps to revenue models and customer support, the answer is yes!

Learn how Concerto can help you.

  • 5
  • 2
Tackle projects and never again get stuck behind a technical roadblock.
Join Now