开发者

VBA Error Handling only works on first pass

My code is:

Sub zaa()
'code to find another open file that has the defined name "data" 
'    - useful where the name changes each month or week

For Each wb In Workbooks
  On Error GoTo abcd
  x = wb.Name
  Workbooks(x).Activate
  If Range("Data").Address <> "" Then y = wb.Name

  Exit Sub

abcd:
Next wb

End Sub

Basic goal to find an Excel file with a specific named range when I know it exists but don't know the file name as it changes each week or month. Aim is to find the file and exit the sub at that point (or then do other code on that file and exit rather than going to other files.)

I find it works okay if I only have two files open but not if have more (unless the target one is second in line). Whilst I can run with what I have I thought others may benefit from what I have and I can have a more robust solution.

UPDATE: Thanks to all those who have responded & to Mitch for putting original post in readable format! (I have since learnt how to correct that issue and to be able to copy code directly - which I have indicated in some comments below that I was having trouble with.) I have had varying degrees of success - code from PaulStock & Reafidy both worked for me originally but PaulStock code has stopped working for me. Responses & code from Jean-François Corbett and Chris Neilsen have been helpful to me but presently when I run the code (as is) it appears to do nothing - ie doesn't leave the sheet I run it from (and that's not where the range name data appears). I was originally running the code in Excel 2007 in a sheet with other macros but to try and get different results have moved them to a stand alone sheet with no other macro files open. Have also tried running them from a file saved as '97-'03 format. Neither yielded different results. Others with more experience than I (see the errors I made evidenced in comments discussion with Reafidy & bear in mind that my original posted code was result of material found through google and modified by me for specific task & application and implication that I am not savvy开发者_如何转开发 enough to have come up with it on my own) may find other solutions better for them but for now:

I am more than happy as Reafidy's code works for me.

As I am not registered (I did try but it didn't work) and don't have enough reputation points I can not vote but I have put a check mark next to Reafidy's solution.

FURTHER UPDATE: I have now discovered that PaulStock & Jean-François Corbett's code wasn't working for me due to a very minor discrepancy on my part - the code contained "Data" whilst my named range was "data". Making this change (to allow for case sensitivity) in their code means that both of their solutions now work for me and hence I have attempted to add a tick to their solutions! Unfortunately I have found that only one solution can have a tick.

I have also learnt that I took Chris' code too literally. In an effort to test each code 'as is' that is what I did. A simple additon of 'wb.activate' in the section where he has 'do stuff' makes the code do what I want.

THANKS again for all four contributions.


You're doing a few convoluted things in your code, and I think that's what's confusing the issue.

Having you error handler (abcd:) start in the middle of a For...Next loop is incredibly bad practice and can only lead to confusion. I can't think of a reason why this should ever be done.

Anyhow, there's no need to use error handling for this task. I understand that there are exceptional cases where bad VBA design forces us to use error handling instead of what should be built-in VBA functionality (e.g. testing whether a non-Variant array has been allocated), but this is not one of those cases. There's a standard way to do it (.Names collection of workbook object), and using error handling instead of this is messy and convoluted and asking for trouble.

Also, why say

x = wb.Name
Workbooks(x).Activate

when you can just say wb.Activate? You're not using x for anything. Or y, for that matter.

The following works and is both simplified and optimised, relative to your original code as well as to the other answers that have been given up to now:

Sub zaa2()
    Dim wb As Workbook
    Dim nm As Name

    For Each wb In Workbooks
        For Each nm In wb.Names
           If nm.Name = "Data" Then
                wb.Activate
                Exit Sub
           End If
        Next
    Next wb
End Sub
' A workbook containing a range named "Data" is now activated 
' (if one is found amongst the open workbooks). 
' Note that there may be more than one, but only the first found is activated...

EDIT: In your comment, you mention you had trouble due to confusion between uppercase "Data" and lowercase "data". To guard against this in the future, one possibility is to ignore case. This could be done as follows:

If StrComp(nm.Name, "data", vbTextCompare) = 0 Then

or

If LCase(nm.Name) = "data" Then

Both will return True if nm.Name is "Data", "data", "dATa", etc.


You can't use error handling like that. Either move the error handling out of the loop or reset the error handler each time it occurs, so use error handling like this:

A much preferred alternative would be:

Sub Test()

    For Each wb In Workbooks
        x = wb.Name
        Workbooks(x).Activate            

        If RangeExists("Data") Then
            y = wb.Name
            Exit Sub
        End If

    Next wb

End Sub
Function RangeExists(s As String) As Boolean
    On Error Resume Next
    RangeExists = Range(s).Count > 0
End Function

EDIT:

@Jean-François Corbett, I have to say you a very quick to jump on the down vote button. The first solution I posted was because I made the assumption that the OP was not posting his entire code hence why I did not attempt to simplify it or "clean it up" like I usually do. I agree I did not word my answer well, but with regard to the first solution I was trying to demonstrate that he needed to reset the error handler. Unfortunately I should have said a "prefered alternative would be".

@Derek, Sorry I was unable to answer your further questions in time. Obviously you are free to choose whatever method you like. In my opinion the multiple loop solution provided by others which digs into the workbook name collection is unnecessary and long winded. Now more importantly the name collection can contain names which refer to a constant, formula, or a range. I presume you want to only check if the defined name is specifically a named range which means the looping method provided by others would need to be adjusted to be reliable.

I agree with comments made by others that error handling should be avoided BUT unnecessary looping in excel can be as much of an evil as using error handling and personally I avoid it like the plague.

The function above can be placed in its own module and be reused as much as you like. It is quick, reliable, avoids unnecessary looping, checks SPECIFICALLY for a named range in a workbook and is the most widely accepted/used method for checking if a named range exists within the excel vba community (by this I mean using a function and error handling over looping through the name collection). Do a google search for "Check If Named Range Exists" if you don't believe me. Or ask at www.ozgrid.com/forum if you want other excel vba experts opinion's.

Now that I know you have posted your entire code and that you did not intend to activate every workbook, you could use this code which will activate the first workbook found with the named range "data":

Sub Test3()
    Dim wbLoop As Workbook

    For Each wbLoop In Workbooks
        If RangeExists("data", wbLoop) Then
            wbLoop.Activate
            Exit Sub
        End If
    Next wbLoop

End Sub
Function RangeExists(s As String, wb As Workbook) As Boolean
   On Error Resume Next
   RangeExists = wb.Names(s).RefersToRange.Count > 0
End Function

I completely understand the need for positive Criticism and I believe in the down vote system if it is used correctly. However, with two down votes for what I believe was a reasonable solution and along with my help with the ops formatting issues - unfortunately I cant help but feel like I want to distance myself from this forum.


Heres an alternative method without getting fancy with the error handler

Sub zaa()
    Dim wb As Workbook
    Dim CheckForNamedRange As Boolean
    Dim nm As Name

    On Error GoTo EH

    For Each wb In Workbooks
        CheckForNamedRange = True
        Set nm = wb.Names("data")
        If CheckForNamedRange Then
            ' Name found
            ' do stuff

            Exit For
        End If
    Next
Exit Sub
EH:
    If CheckForNamedRange Then
        ' Name not found
        Err.Clear
        CheckForNamedRange = False
        Resume Next
    Else
        ' Some other error occured, so handle it
        '...
    End If
End Sub


Try this code. Shouldn't have to worry about getting errors.

Sub zaa()
    For Each wb In Workbooks
        x = wb.Name
        Workbooks(x).Activate
        For Each n In Workbooks(x).Names
           If n.Name = "Data" Then
                y = wb.Name
                Exit Sub
           End If
        Next

    Next wb
End Sub
0

上一篇:

下一篇:

精彩评论

暂无评论...
验证码 换一张
取 消

最新问答

问答排行榜