For Each Loop - Strange Behaviour (logic breakdown)

[vanhunk]

For Each Loop - Strange BehaviourI have a list of names and use the FOR EACH Loop to check if a worksheet exists in the workbook with the same. It works perfectly up to the point where such a worksheet actually exists, it indicates that this worksheet does indeed exist. However, after this point it erroneously indicates the existence of worksheets with the remainder of the names in the list.For example I have a list with the following names: "Name1", "Name2", "Name3", "Name4", "Name5", "Name6", and "Name7". The workbook contains sheets with the names "Name4" and "Name7". The code is suppose to indicate whether sheets with names , "Name1", "Name2", "Name3", "Name4", "Name5", "Name6", and "Name7" exist in the workbook. It does exactly that untill it encounters the name "Name4" in the list. It correctly indicates that sheet "Name4" does indeed exist, but then continue indicating that the sheets "Name5" and "Name6" also exist, which in this case does not.I would appreciate an understanding of this behaviour and how the code should be changed to achieve the intended results.Note: Should the code end up in one line I would appreciate some assistance in correcting it, I have tried everything and every time it ends up in one long line (holding thumbs)!Regards,vanhunk

Sub VerifyCurrentList()  
'Verify that the proposed sheet name does not already exist in the workbook:'------------------------------------------------------------------------------    
Dim strContractor As String    
Dim wks As Worksheet    
Dim bln As Boolean    
Dim oRange As Range    
Dim rCell As Range        
'Define the search range:        
Set ws = Worksheets("Start Sheet")         
Set oRange = ws.Range(Range("ContractorsList").Offset(1, 0), Range("ContractorsList").Offset(30, 0).End(xlUp))            
For Each rCell In oRange        
strContractor = rCell.Value    
'MsgBox strContractor       
On Error Resume Next        
Set wks = ActiveWorkbook.Worksheets(strContractor)        
On Error Resume Next        
If Not wks Is Nothing Then        
bln = True    Else        
bln = False        
Err.Clear    
End If    
If bln = False Then        
MsgBox strContractor & " is not current!"            
Else        
MsgBox strContractor & " is current!"            
End If        
Next rCell
End Sub

[snb]

Sub M_snb() 
    sn=sheets("Start Sheet").range("ContractorsList").Offset(1, 0).specialcells(2)

    for j=1 to ubound(sn)
      msgbox sn(j,1)  & "does" & iif(evaluate("Isref(" & sn(j,1) & "!A1)"),""," not") & " exist"
    Next
End Sub

[vanhunk]

Sub M_snb()      
sn=sheets("Start Sheet").range("ContractorsList").Offset(1, 0).specialcells(2)      
for j=1 to ubound(sn)       
msgbox sn(j,1)  & "does" & iif(evaluate("Isref(" & sn(j,1) & "!A1)"),""," not") & " exist"     
Next 
End Sub

Hi snb I get "Run time error 13: Type mismatch in the msgbox line. Thanks, vanhunk

[snb]

I don't 'get it'.


You can check stepping through the code: F8

[Aflatoon]

To do it the original way, you must set wks = Nothing after you have located it:
[vba] If Not wks Is Nothing Then
bln = True
Set wks = Nothing
Else

'...
[/vba]

[Paul_Hossler]

I don't think your two 'On Error Resume Next' statements are correct

Option Explicit ' <<<< added
Sub VerifyCurrentList()
Dim strContractor As String
Dim wksStart As Worksheet, wksContractor As Worksheet ' <<<< added wksContractor
Dim blnContractorSheetExists As Boolean
Dim oRange As Range
Dim rCell As Range

'Define the search range:
Set wksStart = Worksheets("Start Sheet")

Set oRange = wksStart.Range(Range("ContractorsList").Offset(1, 0), Range("ContractorsList").Offset(30, 0).End(xlUp))

For Each rCell In oRange
strContractor = rCell.Value

'MsgBox strContractor
Set wksContractor = Nothing ' <<<< I'd use a On Error differently
On Error Resume Next
Set wksContractor = ActiveWorkbook.Worksheets(strContractor)
On Error Goto 0

blnContractorSheetExists = Not (wksContractor Is Nothing)

If blnContractorSheetExists Then
MsgBox strContractor & " is current!"
Else
MsgBox strContractor & " is not current!"
End If
Next rCell
End Sub

Paul

[vanhunk]

@Aflatoon Thank you very much, it is what I was looking for. Why is it necessary to do that, I would have thought that

 Set wks = ActiveWorkbook.Worksheets(strContractor)

would have taken care of it? By the way you don't by any change have a solution to me problem in terms of the one-liner? Thanks again, vanhunk

[vanhunk]

@Paul Thank you Paul the code is doing exactly what it is suppose to be doing now. What do you mean by "I'd use an On Error differently"? Do you refer to the change you made, i.e. changing the second "On Error Resume Next" to "On Error Goto 0", or did you have something else in mind? Thanks. Regards, vanhunk

[Paul_Hossler]

Glad it's working

I think the 2 Resume Next's were unneeded (probably didn't hurt), but the Goto 0 was in the wrong place

Mostly matter of style and personal opinion:

I think that the one statement that you really need to catch an error on is the second Set


1. I tend to be very wordy so I like to explicitly Set a condition I can test for (Set ... = Nothing) first

2. Tell Excel to ignore ALL errors (On Error Resume Next)

3. Execute the statement that could cause an error (Set wksContractor ...)

4. Tell Excel to stop ignoring errors (On Error Goto 0)

5. Then test to see if the condition in #1 is still there ( Is Nothing)


In general (i.e. most of the time) I bracket the potential error-generating statement: On Error Resume Next right before, and On Error Goto 0 right after.

You should never just let Excel go blindly along ignoring errors for longer than absolutely necessary

        Set  wksContractor = Nothing ' <<<< I'd use a On Error differently
         On Error Resume Next
         Set wksContractor = ActiveWorkbook.Worksheets(strContractor) 
         On Error Goto 0
        blnContractorSheetExists = Not (wksContractor Is Nothing)

There are more sophisticated and powerful (and complicated) ways to handle errors, but for relatively simple things, I prefer to do it the way above. All the code that could cause an error and what to do if there is one, is in one place

Further un-asked for but volunteered anyway suggestion: You will save yourself a lot of head scratching if you used more descriptive variable names, i.e. blnContractorSheetExists and not just bln. The computer doesn't care, but since you'll have to read the macro later I think you're find it much easier to trace through the logic after 6 months

Paul

[vanhunk]

@Paul Hossler Hi Paul, I like and appreciate your response, have a great day. Regards vanhunk