Deleting rows from a spreadsheet where Column C is blank

Question

I have a script which I use to delete rows from a Excel Sheet, dependent on the Column C cell of that row being blank.

I believe deleting a range might be quicker than deleting individual rows, but I'm unsure.

Sub Shorter()
'47ms to remove blanks rows of 80 rows
'Searches the designated Column and checks for a Blank Cell
'It then Sets a Marker and then continues until it finds a cell with data in it.
'That blank range is then deleted.
 t = GetTickCount
 With ActiveSheet
 Firstrow = 20
 LastRow = .UsedRange.Rows(.UsedRange.Rows.Count).Row
 For i = LastRow To Firstrow Step -1
 If i = StartRow - 1 Then GoTo EndSubAutoDeleteRowsFromEndRow
 If .Cells(i, "C") = Empty Then
 DelRangeStartRow = i
 ii = 0
 Do While .Cells(i - ii, "C") = Empty
 DelRangeEndRow = (i - ii)
 ii = ii + 1
 Loop
 Range(.Rows(DelRangeStartRow), .Rows(DelRangeEndRow)).Delete Shift:=xlUp
 End If
 Next i
 End With
 'Come here if you are finished deleting
 EndSubAutoDeleteRowsFromEndRow:
 SecondsElapsed = GetTickCount - t
End Sub

• Kaz: Whilst CR is the place to come for improving your code (including speed). It is not the place to seek explanation of why your code does what it does. This is the kind of question that belongs on Stack Overflow.
• Kaz: I suspect, however, that it's because the "delete operation" has some level of fixed overhead (I.E. re-formatting the sheet every time it's finished deleting) so calling it once (to delete 5 rows) has less overhead than calling it 5 times (to delete one row each time).
• Kaz: Now that your question has mutated, some advice: Good CR questions are generally of the form: "My is designed to do XYZ, how could it have been done better?". Specifically, "Optimise" is a rather vague term. Optimise for what, specifically?
• Jean-Pierre Oosthuizen: Thanks @Zak, appreciate the feedback. Still trying to figure this site out, which has been frustrating at best
• Kaz: Also, this is a good place to start: Getting the best value out of Code Review - Asking Quesitons

Answer 1

Things I like:

• Going last row to first, so as to avoid messiness with row numbers that occur when deleting whole rows.

• Good use of With

• Good function naming of GetTickCount (at least, assuming it does what it says it does)

---

And now that that's done:

---

Option Explicit

This should be at the top of every code module you ever create in VBA. Go to Tools --> Options --> Require Variable Declaration and it will automatically insert it for you from now on.

This is important because without it, VBA will interpret any new variable names (including mis-spellings) as entirely new variables, instead of what you intended.

It also forces you to declare your variables. So you must explicitly give them a type (Long, String, Variant etc.) and a scope (Procedure Dim var As, Module Private Var As, Project Public Var As).

This will then automatically catch all sorts of unintended situations (such as accidentally setting a number equal to an object) which would not be caught if VBA has to assume that all your variables are Variant because you never explicitly declared them.

---

Naming

Good Variable naming is one of the most important skills you can develop as a developer. Variable names (including values, objects, functions, even whole projects) should be Clear, Concise and, above all, Unambiguous.

Your variable names here are good. Your function name is not. Longer bears absolutely no relation to "Deleting Rows".

A function name like RemoveRowsWithEmptyCellsInRange would be far better.

---

Re-usability

Rather than hard-coding this function for column C of the active sheet, why not create a function to do it for any column at all?

Perhaps something like this?

Private Sub DescriptiveNameHere()
 Dim columnIndex As Long
 columnIndex = 3 '/ Column "C"
 Dim firstRow As Long, lastRow As Long
 firstRow = 20
 lastRow = Cells(Rows.Count, columnIndex).End(xlUp).row
 RemoveRowsWithEmptyCellsInColumn ActiveSheet, columnIndex, firstRow, lastRow
End Sub
Public Sub RemoveRowsWithEmptyCellsInColumn(ByRef targetSheet as Worksheet, ByVal columnIndex As Long, ByVal firstRow As Long, ByVal lastRow As Long)
 Dim ws as Worksheet
 Set ws = targetSheet
 ws.Activate
 Dim row As Long, col As Long
 Dim checkCell As Range
 col = columnIndex
 For row = lastRow To firstRow Step -1
 Set checkCell = ws.Cells(row, col)
 If IsEmpty(checkCell) Then Rows(row).Delete
 '/ You might also want to check for cells that appear empty but contain E.G. a formula that is currently displaying no value, like so:
 ' If checkCell.Text = "" Then Rows(row).Delete
 Next row
End Sub

---

Speed

Very low-hanging standard VBA fruit here:

Public Sub DisableApplicationSettings()
 Application.ScreenUpdating = False
 Application.EnableEvents = False
 Application.Calculation = xlCalculationManual
End Sub
Public Sub ResetApplicationSettings()
 Application.ScreenUpdating = True
 Application.EnableEvents = True
 Application.Calculation = xlCalculationAutomatic
End Sub

Put that at the start and end of any VBA program and it will run inordinantly faster.

However, it doesn't take into account how the spreadsheet was before your program started (what if your user already had calculation set to manual?).

So, these, coupled with some public variables and some error handling so premature ending of the code execution still restores the settings, is even better:

Public VarScreenUpdating As Boolean
Public VarEnableEvents As Boolean
Public VarCalculation As XlCalculation
Public Sub StoreApplicationSettings()
 VarScreenUpdating = Application.ScreenUpdating
 VarEnableEvents = Application.EnableEvents
 VarCalculation = Application.Calculation
End Sub
Public Sub DisableApplicationSettings()
 Application.ScreenUpdating = False
 Application.EnableEvents = False
 Application.Calculation = xlCalculationManual
End Sub
Public Sub RestoreApplicationSettings()
 Application.ScreenUpdating = VarScreenUpdating
 Application.EnableEvents = VarEnableEvents
 Application.Calculation = VarCalculation
End Sub

Answer 2

The code below uses 7 lines verse 17.

It deletes the Rows individually as opposed to deleting a Range of blanks cells.

Unfortunately, it does take a little longer than the original code as posted in the question. So, it will only be better in terms of reducing code length.

Sub Longer()
'140ms to remove blanks rows of 80 rows
'Searches the designated Column and checks for a Blank Cell
'If there is a blank cell it deletes the row
 t = GetTickCount
 With ActiveSheet
 Firstrow = 20
 LastRow = .UsedRange.Rows(.UsedRange.Rows.Count).Row - 1
 For i = LastRow To Firstrow Step -1
 If .Cells(i, "C").Value = Empty Then .Cells(i, "C").EntireRow.Delete
 Next i
 End With
 timetaken = GetTickCount - t
End Sub

Instead of starting another Loop to find the Range of blank cells, rather just delete that row and move onto the next row. This simplifies the reading of the code in terms of understanding the Ranges and the additional Variables.

This code also removes the sort of Error Handling used to cater for the performing the Blank Range check when the For i.... gets to the FirstRow

• Dan: Comments are not the place for extended discussion. Can we move this to chat if its going to go on? We have both the CR Help Desk and 2nd monitor to help discuss improvements to a review at length.
• Kaz: Under normal circumstances, I would consider this a just-about-OK answer and probably not vote on it, but I like to reward people willing to self-improve so here, have an upvote ^^
• ThunderFrame: Using UsedRange won't work if the used range doesn't start on the first row.
• Jean-Pierre Oosthuizen: @ThunderFrame what would the best option be given that my data set doesn't start in the first row or cell?
• ThunderFrame: My answer handles that, and is probably the fastest method here.

Answer 3

Taking Things Further:

One thing that surprised me about both Zak and Jean-Pierre's answers is that neither maximized their range deletion reasoning. Both deleted using (potentially) several stepwise ranges. Here's how I do one deletion no matter what pattern of blanks occurs:

Option Explicit
Sub M1DeleteRowsWhereColCBlank()
'
' M1DeleteRowsWhereColCBlank Macro
'
' Keyboard Shortcut: Ctrl+Shift+Q
'
Dim rngAllData As Range
Dim rngBlankColC As Range
Dim Cell1 As Range
Dim lngLastRow As Long
 Application.ScreenUpdating = False
 Application.Calculation = xlCalculationManual
 With ActiveSheet
 lngLastRow = .Range("A" & .Rows.Count).End(xlUp).Row
 Set rngAllData = .Range(.Cells(20, 3), .Cells(lngLastRow, 3))
 For Each Cell1 In rngAllData
 If Cell1.Value = "" Then
 If rngBlankColC Is Nothing Then
 Set rngBlankColC = Cell1
 Else
 Set rngBlankColC = Union(rngBlankColC, Cell1)
 End If
 End If
 Next Cell1
 If Not (rngBlankColC Is Nothing) Then rngBlankColC.EntireRow.Delete
 End With
 Application.ScreenUpdating = True
 Application.Calculation = xlCalculationAutomatic
End Sub

Some Code Notes:

• I am more flexible about best practices than Zak. If you had to learn from someone, I recommend learning from him. I have outlined our differences below:
  • Zak's points concerning the user's existing preferences and reusability are great, but I will not include them in my answer.
  • One common convention that Zak does not use that I really like is including variable type prefixes. You may find that useful too.
  • I agree with Zak about using descriptive procedure names and variable names. I prefix my macros with M1, M2, etc. to order them in GUI macros tool. I don't name a variable descriptively or with camelCase when I am going to reuse it for many or indiscriminate purposes (like Cell1).
  • You can disable events like Zak does but I do not find the speed advantages to be worth bothering.
  • Zak uses a private and public sub. It's a good practice for parameterization but I don't include them in my answer.
  • As Zak alludes to and I find important enough to include in uncommented code, Excel sometimes cannot detect a cell as empty even when it has nothing in it due to formatting errors or formulas. I actively use Cell.Value = "" to prevent this.
• As you can find discussed online, I do not find UsedRange to be as reliable as I need; .End(xlUp) has some advantages.
• This code was tested on Excel 2010.

I understand that you prefer shorter code, so this may not fit your needs, but I think there are enough potential speed and error prevention improvements in here that future readers may find this answer worthwhile.

• user79074: @zak, thank you for teaching such good practices to people seeking review. I strongly prefer flexibility between good practices and timesaving for myself but I am glad that people can learn from the way you are doing it.
• Jean-Pierre Oosthuizen: Thanks for the answer. Definitely some gems in there for future snd present use
• Kaz: @puzzlepiece87 thanks. Just one thing, I don't use camelCase for my variables? that's news to me!
• user79074: @Zak I am obviously blind :P I must have temporarily been looking at Jean-Pierre's answer when scrolling back and forth. Fixing!
• Kaz: Also, didn't know you could use Union().entirerow like that, good to know.

Answer 4

This code defines a few constants, and uses a With block, but otherwise achieves the result in a single line.

The code would be easier to read and maintain if I broke the line down into 2-3 lines, but I couldn't resist the brevity.

I haven't bothered persisting and restoring the ScreenUpdating, Calculation and EnableEvents properties, because I'm only performing a single operation.

Sub DeleteRows()
 Const ColName As String = "C"
 Const FirstRow As Long = 20
 With ActiveSheet
 .Range(ColName & FirstRow & ":" & ColName & .Range(ColName & .Rows.Count).End(xlUp).Row).SpecialCells(xlCellTypeBlanks).EntireRow.Delete
 End With
End Sub

• Jean-Pierre Oosthuizen: Thanks @ThunderFrame, you answer works perfectly. Time 16ms without disabling the ScreenUpdating. With disabling time is 0ms.
• ThunderFrame: 0ms is impressive, but it does take time... It's just that GetTickCount isn't very high resolution. If you want better timing, have a look at QueryPerfomanceCounter and QueryPerfomanceFrequency functions
• user79074: I should have thought of this, nice work :)