PDA

View Full Version : [SOLVED] What is wrong with delete worksheet?



idnoidno
05-25-2017, 11:42 PM
For Each ws In Sheets
Application.DisplayAlerts = False
If ws.Name = "A" Or "B" Or "C" Then
ws.Delete
End If
Next


I want to delete the sheet's of a or b or c if exit,
but the result is current sheet is deleted.
Is there any mistake?

werafa
05-25-2017, 11:58 PM
try

debug.print ws.name
ws(ws.name).delete

I don't see how this is different, but I'm curious to know what it is you are in fact deleting.
a quick search on the net shows this format is common.

-----

a thought - it could be that in deleting the ws, you are affecting the loop.
you could also try noting the presence of the sheet, and deleting it from outside the loop

do you need to loop to do this?
could you use an on error resume next, and simply delete the sheet whether it exists or not?

Werafa

GTO
05-26-2017, 12:01 AM
Greetings,

It would be helpful if you mentioned that you must have On Error Resume Next someplace before this code. Otherwise, you would be receiving a Type Mismatch error. This also shows why ignoring errors is so very likely to cause issues later in your code, and these issues are hard to track down.

Anyways, it should be more like:


For Each ws In ThisWorkbook.Worksheets
Application.DisplayAlerts = False
If ws.Name = "A" Or ws.Name = "B" Or ws.Name = "C" Then
ws.Delete
End If
Next


Without the 'ws.Name = ' part, you are forcing a binary comparison.

Hope that helps,

Mark

werafa
05-26-2017, 12:05 AM
Thanks Mark
I've learned too

rlv
05-26-2017, 12:12 AM
Another way


Application.DisplayAlerts = False
For Each ws In ThisWorkbook.Worksheets
Select Case ws.Name
Case "A", "B", "C"
ws.Delete
End Select
Next ws
Application.DisplayAlerts = True

GTO
05-26-2017, 12:18 AM
@ werafa:

You are most welcome :)

@ idnoidno:

I missed a bit:



Option Explicit

Sub example()
Dim ws As Worksheet


Application.DisplayAlerts = False

For Each ws In ThisWorkbook.Worksheets
If ws.Name = "A" Or ws.Name = "B" Or ws.Name = "C" Then
ws.Delete
End If
Next

Application.DisplayAlerts = True


End Sub




...in addition to declaring variables, turn off alerts before the loop rather than in it; and always turn alerts back on as soon as appropriate.

Mark

idnoidno
05-26-2017, 12:33 AM
Thanks you all for so soon to provide quick answers,at the same time I also learned the correct code.

werafa
05-26-2017, 12:34 AM
I do agree with the need to manage alerts, on error and such very tightly.
turn it off, do your stuff and turn it back on again - it is there for a reason.

I also like the select case method. it is elegant, and is readable/self explanatory.
Thanks RLV

Paul_Hossler
05-27-2017, 08:42 AM
...in addition to declaring variables, turn off alerts before the loop rather than in it; and always turn alerts back on as soon as appropriate.


I'd have thought that having DisplayAlerts turned off for as little as possible would be better, just like for On Error Resume Next


I normally would do something like this since it's only the .Delete that I want to be 'affected'

I can't imagine that there is any significant performance impact (for this case at least)




Option Explicit

Sub example_2()
Dim ws As Worksheet


For Each ws In ThisWorkbook.Worksheets
Select Case ws.Name
Case "A", "B", "C"
Application.DisplayAlerts = False
ws.Delete
Application.DisplayAlerts = True
End Select
Next
End Sub

werafa
05-28-2017, 02:52 AM
Hi Paul,

I do agree with this approach.
I go as far as indenting the lines for which I am ignoring errors - this makes it easy to see and manage the block of code

idnoidno
05-28-2017, 04:38 AM
I did not expect that a small problem could cause such a big discussion.

GTO
05-29-2017, 07:25 AM
@Paul Hossler:

Hi my friend. I of course agree that if we're only going to be shutting it off and back on 3 times, there'd be no speed difference:blush

My take is that only the .Delete is really in play in the loop, so I generally surround the minimum effected area if that makes sense?

Mark

mdmackillop
05-29-2017, 07:40 AM
Hi Mark
I'd go along with your method.
@ Paul
Maybe no noticeable time difference, but I would avoid the extra steps.

Paul_Hossler
05-29-2017, 07:46 AM
@Paul Hossler:

Hi my friend. I of course agree that if we're only going to be shutting it off and back on 3 times, there'd be no speed difference:blush

My take is that only the .Delete is really in play in the loop, so I generally surround the minimum effected area if that makes sense?

Mark

I agree that in this case bracketing the entire loop with the DisplayAlerts is fine



I just prefer personally / for me / just myself / no on else :devil2: to bracket the minimum since when it gets modified and added to, I I have less chance of making a dumb mistake and leaving something in effect because of a silly oversight.

Just a rule that I like to follow for me (same for On Error Resume Next) UNLESS I think I have a good reason to be a little non-Paul standard


I really only mentioned it since I've seen too many people put something like On Error Resume Next as the first line in the macro and then wonder why something went south and they have no idea where, when, or how

So if the OP had a lot of statements inside the loop, it might not be the best approach to leave the entire For / Next loop bracketed.

GTO
05-29-2017, 07:55 AM
I really only mentioned it since I've seen too many people put something like On Error Resume Next as the first line in the macro and then wonder why something went south and they have no idea where, when, or how...

Sort of like the OP did eh?...:whistle: (See #3)

Hi Malcom:hi: