Problem with Macro
 

I am getting the following error message with this macro: Wrong number of
arguments or invalid property assignment It works fine when I only reference
1 sheet but doesnt run when I reference 2 sheets.

Sub DeleteRowIfZeroInA()
Dim X As Long
Dim R As Range
Dim LastRow As Long
With Worksheets("Sheet3", "Sheet4")
LastRow = .Cells(Rows.Count, "A").End(xlUp).Row
For X = LastRow To 1 Step -1
If .Cells(X, "A").Value = 0 And .Cells(X, "A").Value < "" Then
.Cells(X, "A").EntireRow.Delete xlShiftUp
End If
Next
End With
End Sub
--

Thanks
Bob

Problem with Macro
 

Use a loop:

Sub DeleteRowIfZeroInA()
Dim X As Long
Dim R As Range
Dim LastRow As Long
For Each w In Worksheets(Array("Sheet3", "Sheet4"))
With w
LastRow = .Cells(Rows.Count, "A").End(xlUp).Row
For X = LastRow To 1 Step -1
If .Cells(X, "A").Value = 0 And .Cells(X, "A").Value < "" Then
.Cells(X, "A").EntireRow.Delete xlShiftUp
End If
Next
End With
Next
End Sub
--
Gary''s Student - gsnu200773

Problem with Macro
 

Based on a comment about the speed of the routine I posted back in the "hide
rows with 0 in Column G" thread which I used to develop the original code
for my answer in this thread, I "believe" the code below, incorporated into
the structural change you added to my original code for OP's question, will
be much more efficient (especially if the number of data rows is large)
since it delays interacting with the worksheets until all the rows to be
deleted are grouped into a range of their own...

Sub DeleteRowIfZeroInA()
Dim R As Range
Dim RowsToDelete As Range
Dim W As Worksheet
Dim LastRow As Long
For Each W In Worksheets(Array("Sheet3", "Sheet4"))
With W
LastRow = .Cells(Rows.Count, "A").End(xlUp).Row
For Each R In .Range("A1:A" & CStr(LastRow))
If R.Value = 0 And R.Value < "" Then
If RowsToDelete Is Nothing Then
Set RowsToDelete = R
Else
Set RowsToDelete = Union(R, RowsToDelete)
End If
End If
Next
RowsToDelete.EntireRow.Delete xlShiftUp
Set RowsToDelete = Nothing
End With
Next
End Sub

Rick


"Gary''s Student" wrote in message
...
Use a loop:

Sub DeleteRowIfZeroInA()
Dim X As Long
Dim R As Range
Dim LastRow As Long
For Each w In Worksheets(Array("Sheet3", "Sheet4"))
With w
LastRow = .Cells(Rows.Count, "A").End(xlUp).Row
For X = LastRow To 1 Step -1
If .Cells(X, "A").Value = 0 And .Cells(X, "A").Value < "" Then
.Cells(X, "A").EntireRow.Delete xlShiftUp
End If
Next
End With
Next
End Sub
--
Gary''s Student - gsnu200773

Problem with Macro
 

hi all,

I think that using an Autofilter approach with manual calculation (filter
columns as required, select visible rows, change to manual calc', delete rows
& return to automatic calculation) will be much faster than individually
checking each row. This can be tested on a copy of your data by recording a
macro of these actions & then timing it. It can be more flexible if needed.

Another change which may increase the efficiency of the code that's been
provided is to temporarily change the calculation mode to manual using:
Application.Calculation = xlCalculationManual
& then returning it to automatic at the end using
Application.Calculation = xlCalculationAutomatic


hth
Rob

__________________
Rob Brockett
NZ
Always learning & the best way to learn is to experience...



"Rick Rothstein (MVP - VB)" wrote:

Based on a comment about the speed of the routine I posted back in the "hide
rows with 0 in Column G" thread which I used to develop the original code
for my answer in this thread, I "believe" the code below, incorporated into
the structural change you added to my original code for OP's question, will
be much more efficient (especially if the number of data rows is large)
since it delays interacting with the worksheets until all the rows to be
deleted are grouped into a range of their own...

Sub DeleteRowIfZeroInA()
Dim R As Range
Dim RowsToDelete As Range
Dim W As Worksheet
Dim LastRow As Long
For Each W In Worksheets(Array("Sheet3", "Sheet4"))
With W
LastRow = .Cells(Rows.Count, "A").End(xlUp).Row
For Each R In .Range("A1:A" & CStr(LastRow))
If R.Value = 0 And R.Value < "" Then
If RowsToDelete Is Nothing Then
Set RowsToDelete = R
Else
Set RowsToDelete = Union(R, RowsToDelete)
End If
End If
Next
RowsToDelete.EntireRow.Delete xlShiftUp
Set RowsToDelete = Nothing
End With
Next
End Sub

Rick


"Gary''s Student" wrote in message
...
Use a loop:

Sub DeleteRowIfZeroInA()
Dim X As Long
Dim R As Range
Dim LastRow As Long
For Each w In Worksheets(Array("Sheet3", "Sheet4"))
With w
LastRow = .Cells(Rows.Count, "A").End(xlUp).Row
For X = LastRow To 1 Step -1
If .Cells(X, "A").Value = 0 And .Cells(X, "A").Value < "" Then
.Cells(X, "A").EntireRow.Delete xlShiftUp
End If
Next
End With
Next
End Sub
--
Gary''s Student - gsnu200773

Problem with Macro
 

It would be interesting to see how the two methods compare against each
other (with Calculations and ScreenUpdating both on and off). Back in the
"hide the rows" thread, the OP there ran my old delete line-by-line code
against the Union technique I am proposing here (both with just
ScreenUpdating off) and the time for 65536 rows involved went from 8+
minutes to 4 seconds, so this Union technique seems pretty snappy.

Rick


"broro183" wrote in message
...
hi all,

I think that using an Autofilter approach with manual calculation (filter
columns as required, select visible rows, change to manual calc', delete
rows
& return to automatic calculation) will be much faster than individually
checking each row. This can be tested on a copy of your data by recording
a
macro of these actions & then timing it. It can be more flexible if
needed.

Another change which may increase the efficiency of the code that's been
provided is to temporarily change the calculation mode to manual using:
Application.Calculation = xlCalculationManual
& then returning it to automatic at the end using
Application.Calculation = xlCalculationAutomatic


hth
Rob

__________________
Rob Brockett
NZ
Always learning & the best way to learn is to experience...



"Rick Rothstein (MVP - VB)" wrote:

Based on a comment about the speed of the routine I posted back in the
"hide
rows with 0 in Column G" thread which I used to develop the original code
for my answer in this thread, I "believe" the code below, incorporated
into
the structural change you added to my original code for OP's question,
will
be much more efficient (especially if the number of data rows is large)
since it delays interacting with the worksheets until all the rows to be
deleted are grouped into a range of their own...

Sub DeleteRowIfZeroInA()
Dim R As Range
Dim RowsToDelete As Range
Dim W As Worksheet
Dim LastRow As Long
For Each W In Worksheets(Array("Sheet3", "Sheet4"))
With W
LastRow = .Cells(Rows.Count, "A").End(xlUp).Row
For Each R In .Range("A1:A" & CStr(LastRow))
If R.Value = 0 And R.Value < "" Then
If RowsToDelete Is Nothing Then
Set RowsToDelete = R
Else
Set RowsToDelete = Union(R, RowsToDelete)
End If
End If
Next
RowsToDelete.EntireRow.Delete xlShiftUp
Set RowsToDelete = Nothing
End With
Next
End Sub

Rick


"Gary''s Student" wrote in
message
...
Use a loop:

Sub DeleteRowIfZeroInA()
Dim X As Long
Dim R As Range
Dim LastRow As Long
For Each w In Worksheets(Array("Sheet3", "Sheet4"))
With w
LastRow = .Cells(Rows.Count, "A").End(xlUp).Row
For X = LastRow To 1 Step -1
If .Cells(X, "A").Value = 0 And .Cells(X, "A").Value < "" Then
.Cells(X, "A").EntireRow.Delete xlShiftUp
End If
Next
End With
Next
End Sub
--
Gary''s Student - gsnu200773

Problem with Macro
 

Very nice...but instead of:

RowsToDelete.EntireRow.Delete xlShiftUp

Perhaps:

If RowsToDelete Is Nothing Then
Else
RowsToDelete.EntireRow.Delete xlShiftUp
End If

"just in case..."

--
Gary''s Student - gsnu200773


"Rick Rothstein (MVP - VB)" wrote:

Based on a comment about the speed of the routine I posted back in the "hide
rows with 0 in Column G" thread which I used to develop the original code
for my answer in this thread, I "believe" the code below, incorporated into
the structural change you added to my original code for OP's question, will
be much more efficient (especially if the number of data rows is large)
since it delays interacting with the worksheets until all the rows to be
deleted are grouped into a range of their own...

Sub DeleteRowIfZeroInA()
Dim R As Range
Dim RowsToDelete As Range
Dim W As Worksheet
Dim LastRow As Long
For Each W In Worksheets(Array("Sheet3", "Sheet4"))
With W
LastRow = .Cells(Rows.Count, "A").End(xlUp).Row
For Each R In .Range("A1:A" & CStr(LastRow))
If R.Value = 0 And R.Value < "" Then
If RowsToDelete Is Nothing Then
Set RowsToDelete = R
Else
Set RowsToDelete = Union(R, RowsToDelete)
End If
End If
Next
RowsToDelete.EntireRow.Delete xlShiftUp
Set RowsToDelete = Nothing
End With
Next
End Sub

Rick


"Gary''s Student" wrote in message
...
Use a loop:

Sub DeleteRowIfZeroInA()
Dim X As Long
Dim R As Range
Dim LastRow As Long
For Each w In Worksheets(Array("Sheet3", "Sheet4"))
With w
LastRow = .Cells(Rows.Count, "A").End(xlUp).Row
For X = LastRow To 1 Step -1
If .Cells(X, "A").Value = 0 And .Cells(X, "A").Value < "" Then
.Cells(X, "A").EntireRow.Delete xlShiftUp
End If
Next
End With
Next
End Sub
--
Gary''s Student - gsnu200773

Problem with Macro
 

Very nice...

Thanks... it looks like this Union method should become the method of choice
to recommend to persons asking about deleting non-contiguous rows (or
columns) over the previously recommended (what I'm guessing was the
standard) response of iterating the rows (or columns) backwards and deleting
them one-by-one as they are visited. Since my previous posting, the OP from
the "hide the rows" thread tested the two approaches (although there, the
iterations were not in a backward direction, if that matters)... over 8
minutes to hide the rows one-at-time as they were visited as opposed to 4
seconds using the Union method. I'm guessing the technique will show a
similar vast improvement in times when applied to the "delete the rows" task
as well.

but instead of:

RowsToDelete.EntireRow.Delete xlShiftUp

Perhaps:

If RowsToDelete Is Nothing Then
Else
RowsToDelete.EntireRow.Delete xlShiftUp
End If

"just in case..."

Excellent point!

Rick







Gary''s Student - gsnu200773


"Rick Rothstein (MVP - VB)" wrote:

Based on a comment about the speed of the routine I posted back in the
"hide
rows with 0 in Column G" thread which I used to develop the original code
for my answer in this thread, I "believe" the code below, incorporated
into
the structural change you added to my original code for OP's question,
will
be much more efficient (especially if the number of data rows is large)
since it delays interacting with the worksheets until all the rows to be
deleted are grouped into a range of their own...

Sub DeleteRowIfZeroInA()
Dim R As Range
Dim RowsToDelete As Range
Dim W As Worksheet
Dim LastRow As Long
For Each W In Worksheets(Array("Sheet3", "Sheet4"))
With W
LastRow = .Cells(Rows.Count, "A").End(xlUp).Row
For Each R In .Range("A1:A" & CStr(LastRow))
If R.Value = 0 And R.Value < "" Then
If RowsToDelete Is Nothing Then
Set RowsToDelete = R
Else
Set RowsToDelete = Union(R, RowsToDelete)
End If
End If
Next
RowsToDelete.EntireRow.Delete xlShiftUp
Set RowsToDelete = Nothing
End With
Next
End Sub

Rick


"Gary''s Student" wrote in
message
...
Use a loop:

Sub DeleteRowIfZeroInA()
Dim X As Long
Dim R As Range
Dim LastRow As Long
For Each w In Worksheets(Array("Sheet3", "Sheet4"))
With w
LastRow = .Cells(Rows.Count, "A").End(xlUp).Row
For X = LastRow To 1 Step -1
If .Cells(X, "A").Value = 0 And .Cells(X, "A").Value < "" Then
.Cells(X, "A").EntireRow.Delete xlShiftUp
End If
Next
End With
Next
End Sub
--
Gary''s Student - gsnu200773

Problem with Macro
 

Just a quick follow up. In the "hide" thread (Subject: Code runs to slow),
Peter T mentioned that Union "becomes exponentially slower if/as the number
of discontinuous areas in the unioned range increase" and recommend working
in chunks of unionized groups of, say, 80 to 100 at a time... so that is
something that should probably be factored into the coding in future
postings of this method as well.

Rick


"Rick Rothstein (MVP - VB)" wrote in
message ...
Very nice...

Thanks... it looks like this Union method should become the method of
choice to recommend to persons asking about deleting non-contiguous rows
(or columns) over the previously recommended (what I'm guessing was the
standard) response of iterating the rows (or columns) backwards and
deleting them one-by-one as they are visited. Since my previous posting,
the OP from the "hide the rows" thread tested the two approaches (although
there, the iterations were not in a backward direction, if that
matters)... over 8 minutes to hide the rows one-at-time as they were
visited as opposed to 4 seconds using the Union method. I'm guessing the
technique will show a similar vast improvement in times when applied to
the "delete the rows" task as well.

but instead of:

RowsToDelete.EntireRow.Delete xlShiftUp

Perhaps:

If RowsToDelete Is Nothing Then
Else
RowsToDelete.EntireRow.Delete xlShiftUp
End If

"just in case..."

Excellent point!

Rick







Gary''s Student - gsnu200773


"Rick Rothstein (MVP - VB)" wrote:

Based on a comment about the speed of the routine I posted back in the
"hide
rows with 0 in Column G" thread which I used to develop the original
code
for my answer in this thread, I "believe" the code below, incorporated
into
the structural change you added to my original code for OP's question,
will
be much more efficient (especially if the number of data rows is large)
since it delays interacting with the worksheets until all the rows to be
deleted are grouped into a range of their own...

Sub DeleteRowIfZeroInA()
Dim R As Range
Dim RowsToDelete As Range
Dim W As Worksheet
Dim LastRow As Long
For Each W In Worksheets(Array("Sheet3", "Sheet4"))
With W
LastRow = .Cells(Rows.Count, "A").End(xlUp).Row
For Each R In .Range("A1:A" & CStr(LastRow))
If R.Value = 0 And R.Value < "" Then
If RowsToDelete Is Nothing Then
Set RowsToDelete = R
Else
Set RowsToDelete = Union(R, RowsToDelete)
End If
End If
Next
RowsToDelete.EntireRow.Delete xlShiftUp
Set RowsToDelete = Nothing
End With
Next
End Sub

Rick


"Gary''s Student" wrote in
message
...
Use a loop:

Sub DeleteRowIfZeroInA()
Dim X As Long
Dim R As Range
Dim LastRow As Long
For Each w In Worksheets(Array("Sheet3", "Sheet4"))
With w
LastRow = .Cells(Rows.Count, "A").End(xlUp).Row
For X = LastRow To 1 Step -1
If .Cells(X, "A").Value = 0 And .Cells(X, "A").Value < "" Then
.Cells(X, "A").EntireRow.Delete xlShiftUp
End If
Next
End With
Next
End Sub
--
Gary''s Student - gsnu200773

Problem with Macro
 

And a quick follow up on the quick follow up<g...

I just tested the old method (iterate backwards, deleting one-line at a
time) against the original Union method I proposed in an extreme situation
that should never occur in real life and the code below (which is based on
Peter T's comments). The situation is 40,000 rows of data with every other
row slated for deletion (because the cell in Column A contains a 0); hence
there were 20,000 Areas of one row each. The code below won but "by how
much" varied depending on the contents of the worksheet. First off, one has
to recognize the extremely bizarre nature of this data set. Okay, if the
worksheet only consisted of data, no formulas, the Union method was about 3
times faster than deleting line by line (my originally proposed Union method
took so long I had to stop it manually... so thanks Peter T for noting the
problem with multiple non-contiguous areas); if there was a single column of
formulas (a simple MOD function call), the Union method dropped to only 20%
faster; and if there were 12 columns of formulas (same MOD function call),
then the Union method dropped still further to about 12% faster. Still
though, the time difference in this worst case scenario (20,000 individual
Areas) is significantly better enough to recommend the Union method over the
older one; in normally structured data, where each Area would more than
likely contain multiple cells each, I would expect the Union method would
have come in several magnitudes faster yet. Anyway, here is the code
framework I am suggesting be used to implement this hybrid code meshed
together from my initial proposal and Peter T's comments (it can be embedded
directly within a macros, modified as necessary to fit of course, or bound
into a subroutine of its own to be called from within other macros)...

Dim X As Long
Dim LastRow As Long
Dim RowsToDelete As Range

Const DataStartRow As Long = 1
Const MaxDataColumn As String = "A"
Const SheetName As String = "Sheet1"

With Worksheets(SheetName)
LastRow = .Cells(.Rows.Count, 1).End(xlUp).Row
For X = LastRow To DataStartRow Step -1
' <<Set your test condition here
If .Cells(X, 1).Value = 0 Then
If RowsToDelete Is Nothing Then
Set RowsToDelete = .Cells(X, MaxDataColumn)
Else
Set RowsToDelete = Union(RowsToDelete, .Cells(X, MaxDataColumn))
End If
If RowsToDelete.Areas.Count 100 Then
RowsToDelete.EntireRow.Delete xlShiftUp
Set RowsToDelete = Nothing
End If
End If
Next
End With
If Not RowsToDelete Is Nothing Then
RowsToDelete.EntireRow.Delete xlShiftUp
End If

The worksheet, starting row for the data and the column containing the
maximum rows of data (needed for calculating the LastRow of data) are
defined in the three Const statements. The actual testing is done in the
first If-Then statement inside the For-Next loop. As I have set the code
above up, the test is for a simple "does the cell in column 1 contain 0",
but, of course, this test can be made as complex as required.

Rick


"Rick Rothstein (MVP - VB)" wrote in
message ...
Just a quick follow up. In the "hide" thread (Subject: Code runs to slow),
Peter T mentioned that Union "becomes exponentially slower if/as the
number of discontinuous areas in the unioned range increase" and recommend
working in chunks of unionized groups of, say, 80 to 100 at a time... so
that is something that should probably be factored into the coding in
future postings of this method as well.

Rick


"Rick Rothstein (MVP - VB)" wrote in
message ...
Very nice...

Thanks... it looks like this Union method should become the method of
choice to recommend to persons asking about deleting non-contiguous rows
(or columns) over the previously recommended (what I'm guessing was the
standard) response of iterating the rows (or columns) backwards and
deleting them one-by-one as they are visited. Since my previous posting,
the OP from the "hide the rows" thread tested the two approaches
(although there, the iterations were not in a backward direction, if that
matters)... over 8 minutes to hide the rows one-at-time as they were
visited as opposed to 4 seconds using the Union method. I'm guessing the
technique will show a similar vast improvement in times when applied to
the "delete the rows" task as well.

but instead of:

RowsToDelete.EntireRow.Delete xlShiftUp

Perhaps:

If RowsToDelete Is Nothing Then
Else
RowsToDelete.EntireRow.Delete xlShiftUp
End If

"just in case..."

Excellent point!

Rick







Gary''s Student - gsnu200773


"Rick Rothstein (MVP - VB)" wrote:

Based on a comment about the speed of the routine I posted back in the
"hide
rows with 0 in Column G" thread which I used to develop the original
code
for my answer in this thread, I "believe" the code below, incorporated
into
the structural change you added to my original code for OP's question,
will
be much more efficient (especially if the number of data rows is large)
since it delays interacting with the worksheets until all the rows to
be
deleted are grouped into a range of their own...

Sub DeleteRowIfZeroInA()
Dim R As Range
Dim RowsToDelete As Range
Dim W As Worksheet
Dim LastRow As Long
For Each W In Worksheets(Array("Sheet3", "Sheet4"))
With W
LastRow = .Cells(Rows.Count, "A").End(xlUp).Row
For Each R In .Range("A1:A" & CStr(LastRow))
If R.Value = 0 And R.Value < "" Then
If RowsToDelete Is Nothing Then
Set RowsToDelete = R
Else
Set RowsToDelete = Union(R, RowsToDelete)
End If
End If
Next
RowsToDelete.EntireRow.Delete xlShiftUp
Set RowsToDelete = Nothing
End With
Next
End Sub

Rick


"Gary''s Student" wrote in
message
...
Use a loop:

Sub DeleteRowIfZeroInA()
Dim X As Long
Dim R As Range
Dim LastRow As Long
For Each w In Worksheets(Array("Sheet3", "Sheet4"))
With w
LastRow = .Cells(Rows.Count, "A").End(xlUp).Row
For X = LastRow To 1 Step -1
If .Cells(X, "A").Value = 0 And .Cells(X, "A").Value < "" Then
.Cells(X, "A").EntireRow.Delete xlShiftUp
End If
Next
End With
Next
End Sub
--
Gary''s Student - gsnu200773

Problem with Macro
 

hi Rick,

Since you have an example of an extreme situation would you mind doing some
comparison testing on the speed of my below autofilter solution?
(it's a hybrid too - from various codes of mine)

Sub HowDoesFilteringCompare()
'speed up macro
With Application
.ScreenUpdating = False
.Calculation = xlCalculationManual
End With

Dim LastRow As Long
Dim LastCol As Long
Dim RowsToDelete As Range
Dim RangeOfAutoFilter As Range

Const DataStartRow As Long = 1
Const SheetName As String = "Sheet1"

With Worksheets(SheetName)
'find the "Lastcell" settings based on
http://www.beyondtechnology.com/geeks012.shtml
On Error Resume Next ' in case there are no data cells
LastRow = .Cells.Find(What:="*", SearchDirection:=xlPrevious,
SearchOrder:=xlByRows).Row
LastCol = .Cells.Find(What:="*", SearchDirection:=xlPrevious,
SearchOrder:=xlByColumns).Column
On Error GoTo 0

'check if autofilters are applied, remove & reapply
If .AutoFilterMode Then .AutoFilterMode = False
Set RangeOfAutoFilter = .Range(.Cells(DataStartRow, 1),
..Cells(LastRow, LastCol))

With RangeOfAutoFilter
'filtering column A based on "If .Cells(X, 1).Value = 0"
.AutoFilter Field:=1, Criteria1:="=0"
'selects & deletes all visible rows
On Error Resume Next ' in case there are no visible cells
Set RowsToDelete = .Offset(1, 0).Resize(.Rows.Count -
1).SpecialCells(xlVisible).EntireRow
RowsToDelete.Delete
On Error GoTo 0
'to remove the filters
.AutoFilterMode = False
End With
End With

'free memory
Set RowsToDelete = Nothing
Set RangeOfAutoFilter = Nothing

'reset settings
With Application
.ScreenUpdating = True
.Calculation = xlAutomatic
End With

End Sub


Thanks
Rob

Rob Brockett
NZ
always learning & the best way to learn is to experience...

Problem with Macro
 

Hi Rob, hope you don't mind me butting in!

The Filter method is tried and tested and works well. However somewhat to my
surprise I found the routine posted by Rick worked 30-50% faster than the
filter method (% increasing with greater no. of discontinuous rows to
delete). I disabled screen updating in Rick's.

My test was based on pre-filling data in only col-A like this

Rows.Hidden = False
Columns(1).Clear

Set rng = Range("A1:A10000")

ReDim arr(1 To rng.Rows.Count, 1 To 1) As Long
For i = 1 To UBound(arr) Step 2
arr(i, 1) = 1
Next

rng.Value = arr

I only tested setting the rng to 4k & 10k cells, I didn't bother with 40k as
in another thread.

Comparative speeds may well be different with different data sets and/or
formulas (best also to disable calculation in Rick's if formulas are
included) and not least depending on the 'process', which in this case was
to delete rows.

Couple of comments about your code
It's always good to quote source for code but I'm pretty sure the 'find the
"Lastcell" settings' method was posted in this ng a long time before it may
have appeared on that url.

Although the lastcell method worked fine in my testing with data only in
col-A, typically (though not necessarily) one would want to process a
specific column, eg find zeros in col-A, not in whatever the last data
column might happen to be.

SpecialCells - need to be careful if there is any possibility of attempting
to return 8192+ discontiguous areas, it'll fail so use within a max
cell.count of 16k, if necessary do 'chunks'. However SpecialCells also gets
very slow to return a large qty of discontinuous areas, that alone might be
the reason Rick's worked faster in my testing.

.AutoFilterMode = False

You have that qualified to a range object whereas it should be to a
worksheet, suggest change to

..Parent.AutoFilterMode = False

Just to avoid any misunderstanding, I don't want to give any impression I am
discounting the filter method - I'm not, it's very useful for many purposes.

Regards,
Peter T

"broro183" wrote in message
...
hi Rick,

Since you have an example of an extreme situation would you mind doing
some
comparison testing on the speed of my below autofilter solution?
(it's a hybrid too - from various codes of mine)

Sub HowDoesFilteringCompare()
'speed up macro
With Application
.ScreenUpdating = False
.Calculation = xlCalculationManual
End With

Dim LastRow As Long
Dim LastCol As Long
Dim RowsToDelete As Range
Dim RangeOfAutoFilter As Range

Const DataStartRow As Long = 1
Const SheetName As String = "Sheet1"

With Worksheets(SheetName)
'find the "Lastcell" settings based on
http://www.beyondtechnology.com/geeks012.shtml
On Error Resume Next ' in case there are no data cells
LastRow = .Cells.Find(What:="*", SearchDirection:=xlPrevious,
SearchOrder:=xlByRows).Row
LastCol = .Cells.Find(What:="*", SearchDirection:=xlPrevious,
SearchOrder:=xlByColumns).Column
On Error GoTo 0

'check if autofilters are applied, remove & reapply
If .AutoFilterMode Then .AutoFilterMode = False
Set RangeOfAutoFilter = .Range(.Cells(DataStartRow, 1),
.Cells(LastRow, LastCol))

With RangeOfAutoFilter
'filtering column A based on "If .Cells(X, 1).Value = 0"
.AutoFilter Field:=1, Criteria1:="=0"
'selects & deletes all visible rows
On Error Resume Next ' in case there are no visible cells
Set RowsToDelete = .Offset(1, 0).Resize(.Rows.Count -
1).SpecialCells(xlVisible).EntireRow
RowsToDelete.Delete
On Error GoTo 0
'to remove the filters
.AutoFilterMode = False
End With
End With

'free memory
Set RowsToDelete = Nothing
Set RangeOfAutoFilter = Nothing

'reset settings
With Application
.ScreenUpdating = True
.Calculation = xlAutomatic
End With

End Sub


Thanks
Rob

Rob Brockett
NZ
always learning & the best way to learn is to experience...

Problem with Macro
 

(% increasing with greater no. of discontinuous rows to
delete).

spell checker induced typo !

change
discontinuous
to
discontiguous

Peter T

Problem with Macro
 

The Filter method is tried and tested and works well. However
somewhat to my surprise I found the routine posted by Rick
worked 30-50% faster than the filter method (% increasing with
greater no. of discontinuous rows to delete).

Thanks for jumping in and running your tests; I appreciate that. Also,
thanks for confirming that the "Union Method" is efficient enough to
consider as, if I may be so bold as to extrapolate from your response, a
standard offering to those who ask how to delete rows of data for some given
condition.

I disabled screen updating in Rick's.
..... <snip
best also to disable calculation in Rick's if formulas are included

And here is the problem with my being a (relatively) recent Excel hobbyist
(I started playing with Excel about a year ago after last doing anything
with it back in the early to mid 1990s), I tend to forget things which are
probably obvious to the non-hobby Excel user. I know about turning these
things off, of course, it is just I get so caught up in making code work
(which is pretty much all I had to do in my activities in the compiled VB
newsgroups from which I came) that I tend to not remember to include them.
Turning the screen updating and auto-calculations off dramatically improved
the speed of things... from about 40 minutes with them enabled to 15 minutes
flat with them disabled (for the case of 40,000 rows with 12 columns of
formulas each). So, Peter, thank you once again for pointing me in the right
direction (although unlike your pointer about the slowing down of large
numbers of non-contiguous Areas, this one I should have come up with
myself).

So, incorporating the above into the mix, here is what I am proposing for
the "code framework" for the "Union Method" (again, to be incorporated into
an existing macro or to be made into a stand-alone subroutine as desired)...

Dim X As Long
Dim LastRow As Long
Dim OriginalCalculationMode As Long
Dim RowsToDelete As Range

Const DataStartRow As Long = 1
Const MaxDataColumn As String = "A"
Const SheetName As String = "Sheet1"

On Error GoTo Whoops
OriginalCalculationMode = Application.Calculation
Application.Calculation = xlCalculationManual
Application.ScreenUpdating = False

With Worksheets(SheetName)
LastRow = .Cells(.Rows.Count, 1).End(xlUp).Row
For X = LastRow To DataStartRow Step -1
' <<Set your test condition here
If .Cells(X, 1).Value = 0 Then
If RowsToDelete Is Nothing Then
Set RowsToDelete = .Cells(X, MaxDataColumn)
Else
Set RowsToDelete = Union(RowsToDelete, .Cells(X, MaxDataColumn))
End If
If RowsToDelete.Areas.Count 100 Then
RowsToDelete.EntireRow.Delete
Set RowsToDelete = Nothing
End If
End If
Next
End With
If Not RowsToDelete Is Nothing Then
RowsToDelete.EntireRow.Delete
End If

Whoops:
Application.Calculation = OriginalCalculationMode
Application.ScreenUpdating = True


Rick

Problem with Macro
 

Hello again Rick, you seem to be getting into this larky !

Here's yet another approach, write a helper column to be sorted into rows tb
deleted. Would work well for large data sets without formulas.

A typical reason for deleting rows is to remove duplicates. So instead of
looking for zeros, as in the contrived example, basically the same method
could be used whereby cells in a helper column are "marked" for deletion.
"Marked" cells are sorted into a single block of rows at the bottom for
subsequent deletion. Instead of 'inserting' a helper column as in the demo,
probably faster to use a spare empty column for the helper.

I think this approach should be significantly faster than either the union
(in chunks) or filter methods with large numbers of discontiguous rows to
delete, though either of the other methods might be faster depending on the
scenario. As posted the demo loops 10k cells finding 5k rows tb deleted, but
40/20k only about twice as long (not linear due to insertion of helper
column).

Excuses in advance, cobbled together in haste with possibility of errors!

Sub DelRows_SortMethod()
Dim i As Long, nLastRow As Long, nRow As Long
Dim vArr1
Dim rng As Range
Dim t As Single

Set rng = Range("A10000")
rng.Columns(1).Clear

Range("A1") = "headerA"


ReDim arr(1 To rng.Rows.Count, 1 To 1) As Long
For i = 1 To UBound(arr) Step 2
arr(i, 1) = 1
Next

rng.Value = arr
Erase arr

Stop ' see the column of 0's & 1's
' rows of 0's tb deleted

' above just to set sample data
'''''''''''''''''
' real stuff starts here

t = Timer

Set rng = Range("A2") ' the top cell interest

nLastRow = Cells(Rows.Count, rng.Columns(1).Column).End(xlUp).Row
Set rng = Range(rng, Cells(nLastRow, rng.Column))

vArr1 = rng.Value

ReDim vArr2(1 To UBound(vArr1), 1 To 1)


' "mark" rows tb deleted with a high number (eg the row.count) and other
' rows to keep with consecutive nos to maintain original order
' populate the array with markers (the high no.) & consec' nos.
For i = 1 To UBound(vArr1)
If vArr1(i, 1) = 0 Then
vArr2(i, 1) = UBound(vArr1) + 1
Else
nRow = nRow + 1
vArr2(i, 1) = nRow
End If
Next

If nRow < rng.Rows.Count Then

Columns(rng(1, 2).Column).Insert ' helper column

rng.Offset(, 1).Value = vArr2 ' dump marker's

' sort to put markers at the bottom, ie rows tb deleted
rng.EntireRow.Sort Key1:=rng(1, 2), _
Order1:=xlAscending, _
Header:=xlNo, _
OrderCustom:=1, _
MatchCase:=False, _
Orientation:=xlTopToBottom

' reference the marked rows and delete
rng.Offset(nRow).Resize(rng.Rows.Count - nRow).EntireRow.Delete

' delete the helper column
rng(1, 2).EntireColumn.Delete
Else
' nothing to do

End If

Debug.Print Timer - t
End Sub

This demo inserts the helper column and sorts even if there is only one or a
small number of rows to delete. In practice it would be better to
incorporate something like this -

If nRow < rng.Rows.Count Then
If (rng.Rows.Count - nRow) < [say] 200 Then '
Loop vArr2 from the top down deleting the "marked" rows with v.small
qty
or why not use Union method reading the "marked" array

Else
do as in the demo above


Er, hope that all makes sense....

Regards,
Peter T

Problem with Macro
 

Did I say something about doing things in a hurry!

near the top of the demo please change

Set rng = Range("A10000")
to (say)
Set rng = Range("A1:A10000")

FWIW, I had changed it from A1:A40000 to that single cell in the editor just
before posting.

Regards,
Peter T


"Peter T" wrote in message
<snip
Sub DelRows_SortMethod()
Dim i As Long, nLastRow As Long, nRow As Long
Dim vArr1
Dim rng As Range
Dim t As Single

Set rng = Range("A10000")
rng.Columns(1).Clear

Range("A1") = "headerA"

<snip

Problem with Macro
 

Not important but I really intended not

Set rng = Range("A1:A10000")

but

Set rng = Range("A2:A10000")

IOW, the test range is A2 down to last row in col-A

Peter T

"Peter T" <peter_t@discussions wrote in message
...
Did I say something about doing things in a hurry!

near the top of the demo please change

Set rng = Range("A10000")
to (say)
Set rng = Range("A1:A10000")

FWIW, I had changed it from A1:A40000 to that single cell in the editor
just
before posting.

Regards,
Peter T


"Peter T" wrote in message
<snip
Sub DelRows_SortMethod()
Dim i As Long, nLastRow As Long, nRow As Long
Dim vArr1
Dim rng As Range
Dim t As Single

Set rng = Range("A10000")
rng.Columns(1).Clear

Range("A1") = "headerA"

<snip

Problem with Macro
 

A couple of questions about your suggested approach...

1. Is the time penalty for marking rows and then sorting all the data
(especially if there are formulas involved) really less than accumulating
and deleting whole Areas as they are iterated?

2. Instead of always inserting a helper column, wouldn't it be more
efficient to check if all the columns are in use and, if not, use the
existing empty column to the right of the ending UsedRange column (saving
the insert option only if in the rare case every column is in use)?

Rick


"Peter T" <peter_t@discussions wrote in message
...
Hello again Rick, you seem to be getting into this larky !

Here's yet another approach, write a helper column to be sorted into rows
tb
deleted. Would work well for large data sets without formulas.

A typical reason for deleting rows is to remove duplicates. So instead of
looking for zeros, as in the contrived example, basically the same method
could be used whereby cells in a helper column are "marked" for deletion.
"Marked" cells are sorted into a single block of rows at the bottom for
subsequent deletion. Instead of 'inserting' a helper column as in the
demo,
probably faster to use a spare empty column for the helper.

I think this approach should be significantly faster than either the union
(in chunks) or filter methods with large numbers of discontiguous rows to
delete, though either of the other methods might be faster depending on
the
scenario. As posted the demo loops 10k cells finding 5k rows tb deleted,
but
40/20k only about twice as long (not linear due to insertion of helper
column).

Excuses in advance, cobbled together in haste with possibility of errors!

Sub DelRows_SortMethod()
Dim i As Long, nLastRow As Long, nRow As Long
Dim vArr1
Dim rng As Range
Dim t As Single

Set rng = Range("A10000")
rng.Columns(1).Clear

Range("A1") = "headerA"


ReDim arr(1 To rng.Rows.Count, 1 To 1) As Long
For i = 1 To UBound(arr) Step 2
arr(i, 1) = 1
Next

rng.Value = arr
Erase arr

Stop ' see the column of 0's & 1's
' rows of 0's tb deleted

' above just to set sample data
'''''''''''''''''
' real stuff starts here

t = Timer

Set rng = Range("A2") ' the top cell interest

nLastRow = Cells(Rows.Count, rng.Columns(1).Column).End(xlUp).Row
Set rng = Range(rng, Cells(nLastRow, rng.Column))

vArr1 = rng.Value

ReDim vArr2(1 To UBound(vArr1), 1 To 1)


' "mark" rows tb deleted with a high number (eg the row.count) and
other
' rows to keep with consecutive nos to maintain original order
' populate the array with markers (the high no.) & consec' nos.
For i = 1 To UBound(vArr1)
If vArr1(i, 1) = 0 Then
vArr2(i, 1) = UBound(vArr1) + 1
Else
nRow = nRow + 1
vArr2(i, 1) = nRow
End If
Next

If nRow < rng.Rows.Count Then

Columns(rng(1, 2).Column).Insert ' helper column

rng.Offset(, 1).Value = vArr2 ' dump marker's

' sort to put markers at the bottom, ie rows tb deleted
rng.EntireRow.Sort Key1:=rng(1, 2), _
Order1:=xlAscending, _
Header:=xlNo, _
OrderCustom:=1, _
MatchCase:=False, _
Orientation:=xlTopToBottom

' reference the marked rows and delete
rng.Offset(nRow).Resize(rng.Rows.Count - nRow).EntireRow.Delete

' delete the helper column
rng(1, 2).EntireColumn.Delete
Else
' nothing to do

End If

Debug.Print Timer - t
End Sub

This demo inserts the helper column and sorts even if there is only one or
a
small number of rows to delete. In practice it would be better to
incorporate something like this -

If nRow < rng.Rows.Count Then
If (rng.Rows.Count - nRow) < [say] 200 Then '
Loop vArr2 from the top down deleting the "marked" rows with v.small
qty
or why not use Union method reading the "marked" array

Else
do as in the demo above


Er, hope that all makes sense....

Regards,
Peter T

Problem with Macro
 

"Rick Rothstein (MVP - VB)" wrote in message

I'm guessing then there is no
overall, catch-all "best" solution.

Is there ever !

Regards,
Peter T