Simpler way of coding?

sonicdemonic
06-16-2008, 02:03 PM
Ok, my code copies an existing worksheet. Where the code places it is depending on whether other sheets exist.
Now to test if a worksheet exists I have been using:

Set wSheet = Sheets("Log " & Right(strBoxName, 2))
If Not wSheet Is Nothing Then
'If the sheet log 7 exists then copy.after
End If


Since I have this code checking if many sheets exist I tried to change the format to the following:


If Not Sheets("Log 7") Is Nothing Then
'If the sheet log 7 exists then copy.after
End If


Now that does not work. It returns true reguardless of whether Log 7 exists or not.

I have 5 different worksheets that the user and create more version of and the user can make up to 10 copies of each worksheet. I want the worksheet to be succeeding. The versions that get copied are hidden worksheets that the user does not have acccess to. I tried to have excel copy after the hidden sheet but that doesnt work, somehow it doesnt keep its placing after items are added to the workbook.

Currently the only way I can figure to code this is to set variables for each worksheet I need to test for. Which I can do. I would like to code it so that if will be easier to change in the future.

So my questions are:
Do I have to assign the sheet name I am testing for to a worksheet variable?
Or
I there a better way to to code what I am trying to accomplish?

darkforcesjedi
06-16-2008, 06:19 PM
Function FindSheet(TheName As String) As Boolean
Dim s As WorkSheet
For Each s in Sheets
If s.name = TheName Then
FindSheet = True
Exit For
End If
Next
End Function

shg
06-16-2008, 08:53 PM
Here's another way:
Function SheetExists(sName As String) As Boolean
On Error Resume Next
SheetExists = Len(Worksheets(sName).Name) > 0
End Function

Cas
06-16-2008, 11:33 PM
I assume your original way is using "On Error Resume Next", right? Because accessing a non-existent member of the WorkSheets collection certainly does raise an error...

If you really want to stick with that approach, you can modify it like this:
If ThisWorkbook.Worksheets("Log 7") Is Nothing Then
Else
Debug.Print "sheet does exist"
End If

However, I find this extremely nasty. Looping over the collection and testing each name against the one in question, as demonstrated in darkforcesjedi's example, is much cleaner. And it may even be faster if the number of sheets isn't too large - "On Error Resume Next" comes with a significant performance penalty.

For the specific case you describe, I think I'd modify the approach a little further. Testing worksheet names isn't ideal, because names need to be unique but you want to know if a sheet is one of several copies of a given original, if I understood you correctly. So, I would add a named item (Worksheet.Names.Add(...)) to each copy that identifies the original, and then test against that. :)

shg
06-17-2008, 08:23 PM
I find this extremely nasty ...
Why, nasty, Cas? There are many circumstances when VBA requires explicit error handling.
"On Error Resume Next" comes with a significant performance penalty.
That wasn't obvious to me in a quick test:
Sub test()
Const n As Long = 1000000

Dim i As Long
Dim t As Date

t = Now()

For i = 1 To n
SheetExists1 "Sheet" & CStr(i And 15)
Next i
Debug.Print WorksheetFunction.Text((Now() - t) * 86400 / n, "##0E+0")

End Sub

Function SheetExists1(sName As String) As Boolean
On Error Resume Next
SheetExists1 = Not Worksheets(sName) Is Nothing
End Function

Function SheetExists2(sName As String) As Boolean
On Error Resume Next
SheetExists2 = Len(Worksheets(sName).Name) > 0
End Function

Function SheetExists3(sName As String) As Boolean
Dim wks As Worksheet

For Each wks In Sheets
If wks.Name = sName Then
SheetExists3 = True
Exit For
End If
Next wks
End Function


Performance-wise, on my old laptop, and with 15 sheets named Sheet0 through Sheet15,

SheetExists1 ran in 11us average
SheetExists2 ran in 16us
SheetExists3 ran in 23us

Cas
06-18-2008, 01:34 AM
Why, nasty, Cas? There are many circumstances when VBA requires explicit error handling.
Ah, sorry, I wasn't referring to your suggestion or to using OERN in general, but to relying on how OERN behaves when the error occurs in a control structure line such as an If or For statement, as opposed to an instruction line such as your assignment. I find it non-obvious what will happen when the execution point encounters an Else or Next after that, so it seems much cleaner to me to do the evaluation that might produce an error first, with an explicitly defined default value that will be used if OERN kicks in, and then use that value in the control structure. Which is exactly what your function, used in an If-statement, would in effect do.
That wasn't obvious to me in a quick test:
Interesting. I admit I was just parroting what I had heard/read before. I may have got the wrong end of the stick - the discussion may have only applied to compiled code.

ETA: I re-ran your test for cases in which only some or none of the sheets exist, to see the impact of OENR just being there and actually taking effect. Results below (existent sheets,non-existent sheets @ time in ms)

100000, 0 @ 300 ms
50000, 50000 @ 400 ms
0,100000 @ 440 ms

100000, 0 @ 590 ms
50000, 50000 @ 560 ms
0,100000 @ 460 ms

100000, 0 @ 560 ms
50000, 50000 @ 640 ms
0,100000 @ 680 ms

EZ Archive Ads Plugin for vBulletin Copyright 2006 Computer Help Forum