Time to Execute Code when Deleting Rows

[HangMan]

I have this code which is designed look for two things, either a "-" in column C or a zero value in column F and delete the rows specified in each range and then create a new worksheet based on the value in cell C8. This code works really well, however the problem I have is that I need to apply it to around 1,800 rows to do the same job. When I do so it takes hours to run!

Is there anything that can be changed to make the approach more efficient and speed the code execution up when applied to 180 different ranges? I have no understanding of what causes the code to take so long to run, is it because of the number of rows or for some other reason?

It should be noted that the size of the ranges differs for these sections, between 35, 20, 15 and 10 rows and the For J runs all the way to For J = 1830 to 1820 Step -1 in the full code so these two sections of code are repeated 90 times!

For J = 175 To 145 Step -1
   If .Cells(J, "F") = 0 Then .Rows(J).EntireRow.Delete
Next J

whereas the size of the range is always consistent for these sections

If Cells(139, "C") = "-" Then _
      Cells(137, "C").Resize(8, 1).EntireRow.Delete Shift:=xlUp

This is the structure of the code...

Sub CreateSheets()
Dim arrNames, c
Dim J As Integer

arrNames = Worksheets("Summary").Cells(1).CurrentRegion
Application.ScreenUpdating = False
Worksheets("Template").Visible = True
    For c = LBound(arrNames, 1) + 1 To UBound(arrNames, 1)
            If arrNames(c, 2) = "Complete" Then
                arrNames(c, 2) = "Finished"
                Worksheets("Template").Copy After:=Worksheets(Worksheets.Count)
                    With ActiveSheet
                        .Name = arrNames(c, 1)
                        .Cells(8, "C").Value = .Name
                    End With
                    
                    With ActiveSheet
                    
                        For J = 175 To 145 Step -1
                            If .Cells(J, "F") = 0 Then .Rows(J).EntireRow.Delete
                        Next J
                        
                            If Cells(139, "C") = "-" Then _
                                Cells(137, "C").Resize(8, 1).EntireRow.Delete Shift:=xlUp 
                        
                        For J = 155 To 120 Step -1
                            If .Cells(J, "F") = 0 Then .Rows(J).EntireRow.Delete
                        Next J
                        
                            If Cells(114, "C") = "-" Then _
                                Cells(112, "C").Resize(8, 1).EntireRow.Delete Shift:=xlUp

                        For J = 110 To 75 Step -1
                            If .Cells(J, "F") = 0 Then .Rows(J).EntireRow.Delete
                        Next J
                        
                            If Cells(69, "C") = "-" Then _
                                Cells(67, "C").Resize(8, 1).EntireRow.Delete Shift:=xlUp
                    
                        For J = 63 To 29 Step -1 'FAR
                            If .Cells(J, "E") = 0 Then .Rows(J).EntireRow.Delete
                        Next J
                    
                    End With
            End If
    Next c
        With Worksheets("Summary")
            .Cells(1).CurrentRegion.Value = arrNames
            .Activate
            .Range("A1").Select
        End With
    Worksheets("Template").Visible = False
    Application.ScreenUpdating = True
    
End Sub

Many thanks

[Norie]

What exactly are you trying to do with this code?

What's the purpose of the 'outer' loop that's going through the array arrNames?

[Richard Buttrey]

If your intention is to delete rows that meet some criteria as suggested by your code then you are going about this the wrong way. Using loops like you are doing is just inefficient and particularly so with large ranges.

This subject comes up so often here that it should have it's own FAQ. The fastest way of doing this sort of stuff is to use a standard Data Filter - probably a simple autofilter in a macro. Filter the rows you want to delete then you can use the instruction

Range(your_range).SpecialCells(xlCellTypeVisible).Delete

to delete all the filtered rows in one hit. Then just remove the Autofilter.

Range("A

[HangMan]

Hi Norrie and Richard,

I've attached a sample to hopefully demonstrate what I'm trying to achieve. The code in module 2 is initially generating new worksheets based on the 'Template' tab using a cell value on another worksheet (not included in the attached sample). That part all works fine.

What I've attached shows the original template and one sample sheet that would be created using the code. The template contains blank rows in each section because the number of entries for each newly created tab can vary. Tab AB123 shows the expected result for the first 1,000 or so rows based on the annotation on the Template tab.

No two newly generated sheets will be the same, as in the number of entries will be always be different. The idea is to delete all the rows on each newly created tab once generated where there are zero values or "-" values (as per the annotation). Whilst the code works when there are only a couple of ranges, it falls apart when there are this many rows clearly because as you say my approach is wrong.

I've had a lot of help from this forum in getting the code this far, but always based on a much smaller number of rows and ranges.

Richard I'm unsure how I would go about applying a simple autofilter to achieve this, can it be done in the way you are describing based on the attached?

I need to find a solution to remove all the required rows from each new tab that will be created. So an alternative way to do this part for each range.

For J = 155 To 120 Step -1
    If .Cells(J, "F") = 0 Then .Rows(J).EntireRow.Delete
Next J
                        
If Cells(114, "C") = "-" Then _
    Cells(112, "C").Resize(8, 1).EntireRow.Delete Shift:=xlUp

Many thanks

[Richard Buttrey]

Hmmm,

I'm assuming that the Template sheet is meant to be basic data from which you want to extract a sub set of data. If that's the case then you are making life rather difficult for yourself.

But rather than jumping to conclusions can you explain what you are trying to achieve. By that I don't mean tell us what your existing code is attempting to do I mean explain what data you start with and how you get it (manually input or from some upload process) and what you want to end up with. If we can understand the overall goal then we may be able to suggest better ways of achieving it.

Are you open to changing the layout of your basic data?

[xladept]

Hi Hangman,

My idea is to process the template once with a surrogate named TCCopy - But it uses what "the experts" call spaghetti code

Sub CreateSheetsHMII()
Dim arrNames, c, U As Range, D As Range
Dim J As Long, jf As Long, je As Long, k As Long
                  Application.ScreenUpdating = False
                 Worksheets("Template").Visible = True
    Worksheets("Template").Copy After:=Worksheets(Worksheets.count)
                   ActiveSheet.Name = TCCopy
                  Worksheets("Template").Visible = False

                    With ActiveSheet
                      jf = 175: je = 145: k = 137: GoSub SetD
                      jf = 155: je = 120: k = 114: GoSub SetD
                      jf = 110: je = 75: k = 69: GoSub SetD
                      jf = 63: je = 29: k = 69: GoSub SetD      'FAR
                    End With: D.EntireRow.Delete: GoTo MakeSheets
                    
                    Exit Sub
    
SetD:
    
                    For J = jf To je Step -1
                        If .Cells(J, "F") = 0 Then
                            Set D = .Cells(J, "F"): GoSub SetU
                        End If: Next J
                        
                        If .Cells(k, "C") = "-" Then
            Set D = .Cells(k - 2, "C"): GoSub SetU: End If: Return
    
SetU: If U Is Nothing Then Set U = D Else Set U = Union(D, U): Return

MakeSheets:
arrNames = Worksheets("Summary").Cells(1).CurrentRegion


    For c = LBound(arrNames, 1) + 1 To UBound(arrNames, 1)
            If arrNames(c, 2) = "Complete" Then
                arrNames(c, 2) = "Finished"
                Worksheets("TCCopy").Copy After:=Worksheets(Worksheets.count)
                    With ActiveSheet
                        .Name = arrNames(c, 1)
                        .Cells(8, "C").Value = .Name
                    End With
            End If
    Next c: Sheets("TCCopy").Delete
    
        With Worksheets("Summary")
            .Cells(1).CurrentRegion.Value = arrNames
            .Activate
            .Range("A1").Select
        End With
    
    Application.ScreenUpdating = True
End Sub

[HangMan]

Hi xladept,

I ran your code but it gave me a compile error, highlighting the word .cells shown below in red. The error says Invalied Unqualified Reference. I'm unsue what that means. I note that your code runs from row 29 to row 175. On my main file the rows to delete will be in the range F29:F2430, would you anticipate the code running into similar problems in terms of the time it takes to execute for a range of this size which is the main problem I'm experiencing now?

For J = jf To je Step -1
     If .Cells(J, "F") = 0 Then
     Set D = .Cells(J, "F"): GoSub SetU
End If: Next J

Many thanks

[HangMan]

Hi Richard,

The file itself (as in my main file) contains several data sheets all containing specific data from different sources which is used to populate the ‘template’ (on my working version these data sheets are all contained within the same workbook, and will be hidden).

The template is key to the process and the data pulled into the template updates based on the entry in cell C8 using a variety of Excel formula. Imagine having say 1,500 different people and this template pulling through their bank statement transactions for the last three years and splitting the spend into different categories. This is basically what the template is doing. When you change the entry in C8 (e.g. the person’s name) this will update every single field on the template to reflect that person’s last three years transactions.

The template is designed to accommodate the largest number of entries under any single category, i.e. each person will have a different number of transactions for each heading.

So say row 67 is titled petrol, then rows 69 to 72 provide a summary of every transaction where that person bought petrol and then details these transactions in rows 75 to 110. Person A may have 5 transactions, person B 18, person C 35 and so on.

The header and breakdown for each category is repeated in the rows below for each category. There are 90 categories in total.

When the macro is run it creates a new worksheet for each identified person (based on whether that person shows a ‘complete’ status on another data sheet), names each worksheet using the name in cell C8 resulting in a detailed breakdown of each person’s transactions. Because no two people will ever have the same number of transactions this results in a lot of ‘empty’ rows.

The new worksheets created have to be reviewed and it is not practical for the person reviewing them to have to scroll through rows and rows containing no date so the objective is to delete these rows, just leaving the valid entries for each section.

So far the code pulls in the data and creates the new worksheets very well and efficiently and I would like to keep this functionality as is. The ‘issue’ is the process used to then delete the rows without any entries. This was initially tested with just a small handful of rows and the approach seemed to work well.

For J = 155 To 120 Step -1
     If .Cells(J, "F") = 0 Then .Rows(J).EntireRow.Delete
Next J
                        
     If Cells(114, "C") = "-" Then _
        Cells(112, "C").Resize(8, 1).EntireRow.Delete Shift:=xlUp

For J = 110 To 75 Step -1
     If .Cells(J, "F") = 0 Then .Rows(J).EntireRow.Delete
Next J
                        
     If Cells(69, "C") = "-" Then _
        Cells(67, "C").Resize(8, 1).EntireRow.Delete Shift:=xlUp
                    
For J = 63 To 29 Step -1
    If .Cells(J, "E") = 0 Then .Rows(J).EntireRow.Delete
Next J

But when I then expanded this to accommodate all 2,500 rows it just grinds to a complete halt. I’m afraid this is where my knowledge is lacking, I am still very new to VBA and don’t have a good enough understanding to know how to resolve this problem, so am looking for some help to find a way/efficient process to delete the ‘empty’ rows in each section when the code runs thereby producing readable reports which can be reviewed.

I hope that helps to explain what I’m trying to achieve but please let me know if any of this doesn’t make sense.

Many thanks

[Richard Buttrey]

Hi,

What I was trying to explain was that the basic layout of your data is the problem. All data should be on a single sheet in normalised 2 dimensional table consisting of column label headers and rows that contain individual unique records. One way or another I would urge you to bring your data together into this data table arrangement. Once you have this then any analysis will be simplified considerably.

Upload your workbook, or at least a representative cuts down version of it and we should be able to help further by pointing out what teh datas hould look like.

[HangMan]

Hi Richard,

The source data whilst on several sheets does all conform to the normalised 2 dimensional format consisting of column label headers and rows that contain individual unique records but it would be both impractical and vitrually impossible to combine it all onto a single sheet without hundreds of hours of manual intervention (which would be self defeating) which could lead to lots of potential errors. Each data source consists of hundreds of thousands of rows of data and the data is updated daily. If you can imagine several different finance systems all outputting different information in different formats, this is what I am dealing with.

I don't have an issue with the different data sources, this is perfectly normal in my experience and pulling the data into the template is not the problem at all and doesn't impact in any way and that process works very effectively and quickly and I'm very happy with the way that works.


I've attached a dramatically simplified sample with three tabs. (I can't upload my main workbook owing to the sensitivity of the data). The 'Data' tab contains the source data (which on my main file would come from the several different sheets), the 'Summary' Tab contains the details for which new worksheets can be created. If column B says 'Complete' then the code will create a new worksheet for each and populate it with data from the 'Data' tab, based on the code in cell C8.

If you run the macro called 'Create Sheets' you will see it creates 3 new worksheets with populated data (once run the codes on the 'Data' tab which previously said 'Complete' now say 'Finished'), however in column E on all three new sheets there are zero values and it is these rows I would like to delete when the worksheets are created.

If you then close and re-open the workbook without saving it and this time run the macro called 'Create Sheets and Delete Rows' you will see it does the same thing, but this time it deletes the rows on the new worksheets containing a zero value.

This is exactly what I'm wanting to achieve only on a much larger scale and with varying ranges, as per the previous sample, but the code that is deleting the rows with a zero value whilst it works when there are only a small number of rows to analyse it takes hours when analysing thousands of rows, so clearly needs a different approach as you pointed out.

If deleting rows is not practical a possible alternative would be to 'hide' the rows with a zero value, but that would be a fallback position.

Although much simplified, the basic functionality is the same and hopefully this helps to demonstrate what I'm trying to achieve a little more clearly.

I hope that helps.

Many thanks

[xladept]

It's just as well - I had an error (a minor detail) - try it now

Sub CreateSheetsHMII()
Dim arrNames, c, U As Range, D As Range
Dim J As Long, jf As Long, je As Long, k As Long
            Application.ScreenUpdating = False
                    Worksheets("Template").Visible = True
        Worksheets("Template").Copy After:=Worksheets(Worksheets.count)
                    ActiveSheet.Name = TCCopy
                  Worksheets("Template").Visible = False
                     
                      jf = 175: je = 145: k = 137: GoSub SetD
                      jf = 155: je = 120: k = 114: GoSub SetD
                      jf = 110: je = 75: k = 69: GoSub SetD
                      jf = 63: je = 29: k = 69: GoSub SetD      'FAR
                        U.EntireRow.Delete: GoTo MakeSheets
                        
                    Exit Sub
SetD:
                    For J = jf To je Step -1
                        If Cells(J, "F") = 0 Then
                            Set D = Cells(J, "F"): GoSub SetU
                        End If: Next J
                        
                        If Cells(k, "C") = "-" Then
            Set D = .Cells(k - 2, "C"): GoSub SetU: End If: Return
    
SetU: If U Is Nothing Then Set U = D Else Set U = Union(D, U): Return

MakeSheets:
arrNames = Worksheets("Summary").Cells(1).CurrentRegion

    For c = LBound(arrNames, 1) + 1 To UBound(arrNames, 1)
            If arrNames(c, 2) = "Complete" Then
                arrNames(c, 2) = "Finished"
                Worksheets("TCCopy").Copy After:=Worksheets(Worksheets.count)
                    With ActiveSheet
                        .Name = arrNames(c, 1)
                        .Cells(8, "C").Value = .Name
                    End With
            End If
    Next c: Sheets("TCCopy").Delete
    
        With Worksheets("Summary")
            .Cells(1).CurrentRegion.Value = arrNames
            .Activate
            .Range("A1").Select
        End With
    
    Application.ScreenUpdating = True
End Sub

I note that your code runs from row 29 to row 175. On my main file the rows to delete will be in the range F29:F2430, would you anticipate the code running into similar problems in terms of the time it takes to execute for a range of this size which is the main problem I'm experiencing now?

The difference is that we're only doing it once on the surrogate Template "TCCopy" so, no matter how many rows (and we're only talking about 2500 rows) - it has only to do it once

[HangMan]

Hi xladept,

Apologies for the delay in responding, the forum is once again not sending me notifications as to when there is a response to a post and then about a week or two later I get a whole bunch of emails all in one go for resplies from weeks ago... I'll give this a test and let you know how I get on...

Many thanks for taking the time to look at it for me...

[xladept]

You're welcome, you should have no problem filling in the constants from 2500 down