Reply
 
LinkBack Thread Tools Search this Thread Display Modes
  #1   Report Post  
Posted to microsoft.public.excel.programming
Bob Bob is offline
external usenet poster
 
Posts: 972
Default 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
  #2   Report Post  
Posted to microsoft.public.excel.programming
external usenet poster
 
Posts: 11,058
Default 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
  #3   Report Post  
Posted to microsoft.public.excel.programming
external usenet poster
 
Posts: 1
Default 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


  #4   Report Post  
Posted to microsoft.public.excel.programming
external usenet poster
 
Posts: 32
Default 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



  #5   Report Post  
Posted to microsoft.public.excel.programming
external usenet poster
 
Posts: 1
Default 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






  #6   Report Post  
Posted to microsoft.public.excel.programming
external usenet poster
 
Posts: 11,058
Default 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



  #7   Report Post  
Posted to microsoft.public.excel.programming
external usenet poster
 
Posts: 1
Default 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




  #8   Report Post  
Posted to microsoft.public.excel.programming
external usenet poster
 
Posts: 1
Default 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




Powered by vBulletin® Copyright ©2000 - 2025, Jelsoft Enterprises Ltd.
Copyright ©2004-2025 ExcelBanter.
The comments are property of their posters.
 

About Us

"It's about Microsoft Excel"