Home |
Search |
Today's Posts |
|
#1
![]()
Posted to microsoft.public.excel.programming
|
|||
|
|||
![]()
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
![]()
Posted to microsoft.public.excel.programming
|
|||
|
|||
![]()
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
![]()
Posted to microsoft.public.excel.programming
|
|||
|
|||
![]()
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
![]()
Posted to microsoft.public.excel.programming
|
|||
|
|||
![]()
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
![]()
Posted to microsoft.public.excel.programming
|
|||
|
|||
![]()
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
![]()
Posted to microsoft.public.excel.programming
|
|||
|
|||
![]()
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
![]()
Posted to microsoft.public.excel.programming
|
|||
|
|||
![]()
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
![]()
Posted to microsoft.public.excel.programming
|
|||
|
|||
![]()
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 |
Reply |
Thread Tools | Search this Thread |
Display Modes | |
|
|