Home |
Search |
Today's Posts |
#1
Posted to microsoft.public.excel.programming
|
|||
|
|||
Please take a look at this working code and critque it.
This code is working, but I have the feeling its not the best code.
Basically, I have a sheet in which if column D is equal to zero, I want to delete the row and shift the rows up. I want to do this for every row, starting at row three to bypass the headings. Please let me know your thoughts. With objWkb.Worksheets("Sorted Rankings") TotalRows = objWkb.Worksheets("Sorted Rankings").Range("D" & objWkb.Worksheets("Sorted Rankings").Rows.Count).End(xlUp).Row For Row = 3 To TotalRows Step 1 If Row TotalRows Then GoTo EndNow: If .Cells(Row, "D").Value = 0 Then .Rows(Row).EntireRow.Delete ' Delete Shift:=xlUp Row = Row - 1 ' Because we deleted a row and shifted it up, we have to make sure variable Row does not get increased otherwise the next row down from a deleted row will not be evaluated! End If 'Check TotalRows in case a row was deleted TotalRows = objWkb.Worksheets("Sorted Rankings").Range("D" & objWkb.Worksheets("Sorted Rankings").Rows.Count).End(xlUp).Row Next Row EndNow: |
#2
Posted to microsoft.public.excel.programming
|
|||
|
|||
Please take a look at this working code and critque it.
When deleting rows - you need to work from the bottom up...
For Row = TotalRows to 3 Step -1 -- steveB Remove "AYN" from email to respond "BerkshireGuy" wrote in message oups.com... This code is working, but I have the feeling its not the best code. Basically, I have a sheet in which if column D is equal to zero, I want to delete the row and shift the rows up. I want to do this for every row, starting at row three to bypass the headings. Please let me know your thoughts. With objWkb.Worksheets("Sorted Rankings") TotalRows = objWkb.Worksheets("Sorted Rankings").Range("D" & objWkb.Worksheets("Sorted Rankings").Rows.Count).End(xlUp).Row For Row = 3 To TotalRows Step 1 If Row TotalRows Then GoTo EndNow: If .Cells(Row, "D").Value = 0 Then .Rows(Row).EntireRow.Delete ' Delete Shift:=xlUp Row = Row - 1 ' Because we deleted a row and shifted it up, we have to make sure variable Row does not get increased otherwise the next row down from a deleted row will not be evaluated! End If 'Check TotalRows in case a row was deleted TotalRows = objWkb.Worksheets("Sorted Rankings").Range("D" & objWkb.Worksheets("Sorted Rankings").Rows.Count).End(xlUp).Row Next Row EndNow: |
#3
Posted to microsoft.public.excel.programming
|
|||
|
|||
Please take a look at this working code and critque it.
Couple of comments:
1) It's nearly always a bad idea to use reserved words as variable names (e.g., "Row"). It can get confusing, especially months down the road. 2) You can avoid all the messy row handling if you go backwards through the rows: For nRow = nTotalRows To 3 Step -1 If Cells(nRow, "D").Value = 0 Then _ Rows(nRow).EntireRow.Delete Next nRow 3) You can gain a lot of efficiency if there are a large number of rows to delete by identifying them all and deleting them all at once (e.g., XL doesn't have to reindex each time). Here's one way I might rewrite your code: Dim rCell As Range Dim rDel As Range With objWkb.Worksheets("Sorted Rankings") For Each rCell In .Range("D3:D" & _ .Range("D" & .Rows.Count).End(xlUp).Row) With rCell If .Value = 0 Then If rDel Is Nothing Then Set rDel = .Cells Else Set rDel = Union(rDel, .Cells) End If End If End With Next rCell If Not rDel Is Nothing Then rDel.EntireRow.Delete End With In article .com, "BerkshireGuy" wrote: This code is working, but I have the feeling its not the best code. Basically, I have a sheet in which if column D is equal to zero, I want to delete the row and shift the rows up. I want to do this for every row, starting at row three to bypass the headings. Please let me know your thoughts. With objWkb.Worksheets("Sorted Rankings") TotalRows = objWkb.Worksheets("Sorted Rankings").Range("D" & objWkb.Worksheets("Sorted Rankings").Rows.Count).End(xlUp).Row For Row = 3 To TotalRows Step 1 If Row TotalRows Then GoTo EndNow: If .Cells(Row, "D").Value = 0 Then .Rows(Row).EntireRow.Delete ' Delete Shift:=xlUp Row = Row - 1 ' Because we deleted a row and shifted it up, we have to make sure variable Row does not get increased otherwise the next row down from a deleted row will not be evaluated! End If 'Check TotalRows in case a row was deleted TotalRows = objWkb.Worksheets("Sorted Rankings").Range("D" & objWkb.Worksheets("Sorted Rankings").Rows.Count).End(xlUp).Row Next Row EndNow: |
#4
Posted to microsoft.public.excel.programming
|
|||
|
|||
Please take a look at this working code and critque it.
nitpicking... <g using a union can get slow if the union has many areas.. and will get impossibly slow around 800 areas and more. better to "flush" the range if the area count gets 'too high'. Sub foo() Dim rCell As Range Dim rDel As Range With objWkb.Worksheets("Sorted Rankings") For Each rCell In .Range("D3:D" & _ .Range("D" & .Rows.Count).End(xlUp).Row) With rCell If .Value = 0 Then If rDel Is Nothing Then Set rDel = .Cells Else Set rDel = Union(rDel, .Cells) If rDel.Areas.Count 200 Then Call Flush(rDel) End If End If End With Next rCell End With Call Flush(rDel) End Sub Private Sub Flush(ByRef rDel As Range) If Not rDel Is Nothing Then Application.ScreenUpdating = False rDel.EntireRow.Delete Application.ScreenUpdating = True Set rDel = Nothing End If End Sub -- keepITcool | www.XLsupport.com | keepITcool chello nl | amsterdam JE McGimpsey wrote : Couple of comments: 1) It's nearly always a bad idea to use reserved words as variable names (e.g., "Row"). It can get confusing, especially months down the road. 2) You can avoid all the messy row handling if you go backwards through the rows: For nRow = nTotalRows To 3 Step -1 If Cells(nRow, "D").Value = 0 Then _ Rows(nRow).EntireRow.Delete Next nRow 3) You can gain a lot of efficiency if there are a large number of rows to delete by identifying them all and deleting them all at once (e.g., XL doesn't have to reindex each time). Here's one way I might rewrite your code: Dim rCell As Range Dim rDel As Range With objWkb.Worksheets("Sorted Rankings") For Each rCell In .Range("D3:D" & _ .Range("D" & .Rows.Count).End(xlUp).Row) With rCell If .Value = 0 Then If rDel Is Nothing Then Set rDel = .Cells Else Set rDel = Union(rDel, .Cells) End If End If End With Next rCell If Not rDel Is Nothing Then rDel.EntireRow.Delete End With In article .com, "BerkshireGuy" wrote: This code is working, but I have the feeling its not the best code. Basically, I have a sheet in which if column D is equal to zero, I want to delete the row and shift the rows up. I want to do this for every row, starting at row three to bypass the headings. Please let me know your thoughts. With objWkb.Worksheets("Sorted Rankings") TotalRows = objWkb.Worksheets("Sorted Rankings").Range("D" & objWkb.Worksheets("Sorted Rankings").Rows.Count).End(xlUp).Row For Row = 3 To TotalRows Step 1 If Row TotalRows Then GoTo EndNow: If .Cells(Row, "D").Value = 0 Then .Rows(Row).EntireRow.Delete ' Delete Shift:=xlUp Row = Row - 1 ' Because we deleted a row and shifted it up, we have to make sure variable Row does not get increased otherwise the next row down from a deleted row will not be evaluated! End If 'Check TotalRows in case a row was deleted TotalRows = objWkb.Worksheets("Sorted Rankings").Range("D" & objWkb.Worksheets("Sorted Rankings").Rows.Count).End(xlUp).Row Next Row EndNow: |
#5
Posted to microsoft.public.excel.programming
|
|||
|
|||
Please take a look at this working code and critque it.
Excellent! Thanks everyone.
-Brian |
#6
Posted to microsoft.public.excel.programming
|
|||
|
|||
Please take a look at this working code and critque it.
KeepitCool -
Your code is generating a Method or data member not found error. Thanks Brian |
#7
Posted to microsoft.public.excel.programming
|
|||
|
|||
Please take a look at this working code and critque it.
And much more mundane...
If the cell is empty, then its value will be 0, too. You may want to watch out for that, too. BerkshireGuy wrote: This code is working, but I have the feeling its not the best code. Basically, I have a sheet in which if column D is equal to zero, I want to delete the row and shift the rows up. I want to do this for every row, starting at row three to bypass the headings. Please let me know your thoughts. With objWkb.Worksheets("Sorted Rankings") TotalRows = objWkb.Worksheets("Sorted Rankings").Range("D" & objWkb.Worksheets("Sorted Rankings").Rows.Count).End(xlUp).Row For Row = 3 To TotalRows Step 1 If Row TotalRows Then GoTo EndNow: If .Cells(Row, "D").Value = 0 Then .Rows(Row).EntireRow.Delete ' Delete Shift:=xlUp Row = Row - 1 ' Because we deleted a row and shifted it up, we have to make sure variable Row does not get increased otherwise the next row down from a deleted row will not be evaluated! End If 'Check TotalRows in case a row was deleted TotalRows = objWkb.Worksheets("Sorted Rankings").Range("D" & objWkb.Worksheets("Sorted Rankings").Rows.Count).End(xlUp).Row Next Row EndNow: -- Dave Peterson |
#8
Posted to microsoft.public.excel.programming
|
|||
|
|||
Please take a look at this working code and critque it.
possibly. it was untested and adapted from JE McGimpsey, just to illustrate the flush 'technique'. the objwkb is missing.. thats all, rest works for me. include dave peterson's suggestion With rCell If Len(.Value) 0 And .Value = 0 Then and WHERE does it generate that error? -- keepITcool | www.XLsupport.com | keepITcool chello nl | amsterdam BerkshireGuy wrote : KeepitCool - Your code is generating a Method or data member not found error. Thanks Brian |
Reply |
Thread Tools | Search this Thread |
Display Modes | |
|
|
Similar Threads | ||||
Thread | Forum | |||
VB Code Is Not Working | Excel Discussion (Misc queries) | |||
Code not working and can't see why | Excel Discussion (Misc queries) | |||
Wht is this Code not Working ? | Excel Programming | |||
Code not working | Excel Programming | |||
Code not working | Excel Programming |