What is wrong with delete worksheet?

[idnoidno]

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]

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]

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]

Thanks Mark
I've learned too

[rlv]

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]

@ 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]

Thanks you all for so soon to provide quick answers,at the same time I also learned the correct code.

[werafa]

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]

...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]

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]

I did not expect that a small problem could cause such a big discussion.

[GTO]

@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

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]

Hi Mark
I'd go along with your method.
@ Paul
Maybe no noticeable time difference, but I would avoid the extra steps.

[Paul_Hossler]

@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

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

[OPINION]

I just prefer personally / for me / just myself / no on else 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.

[/OPINION]

[GTO]

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?... (See #3)

Hi Malcom