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
精彩评论