Return to Revisions

You're working off Selection. That's "good enough" for macro-recorder code, which mimicks a user's interactions with a worksheet.

VBA code can do that, sure. But why merely mimick a user's interactions with a worksheet when you have the entire Excel object model in your hands?

But before we look at different/better ways to accomplish what this code does, let's take a look at what it's doing:

• h is initialized at an arbitrary value of 1000. See this SO answer for a very detailed explanation of various ways to locate the last row that contains data on a worksheet. For example this would set h to the last row with data in column A:

   h = ActiveSheet.Range("A" & ActiveSheet.Rows.Count).End(xlUp).Row
  

  By working off the actual last row, you avoid extraneous iterations later, and avoid having to modify your code if/when the worksheet starts containing more than 1000 rows.

• We Select the first cell in the 999th row, and then start looping from 1 to 999.

  If the ActiveCell is empty, we move up a row; else, we... wait a minute... we reassign the loop variable back to 999??

  It's pretty hard to understand what's happening here, and more importantly, why - and explaining why something is happening is exactly what comments are for!

   'this loop sets n to the last row with data (right?)
  

Okay, let's stop here, and look again at what we're trying to accomplish.

We want to delete rows where column A is empty. By adapting this answer to Conditionally deleting rows in a worksheet, we can enormously simplify what's going on here - the key being this little function:

Private Function CombineRanges(ByVal source As Range, ByVal toCombine As Range) As Range
    If source Is Nothing Then
        Set CombineRanges = toCombine
    Else
        Set CombineRanges = Union(source, toCombine)
    End If
End Function

Equipped with this simple tool, we can now iterate our rows once, "union" all empty cells, and delete them all at once:

Public Sub CleanUpActiveSheet()
    
    Dim target As Worksheet
    Set target = ActiveSheet
            
    Dim lastRow As Long
    lastRow = target.Range("A" & target.Rows.Count).End(xlUp).Row

    Dim toDelete As Range
    
    Dim currentRow As Long
    For currentRow = 1 To lastRow
        If IsEmpty(target.Cells(currentRow, 1)) Then
            Set toDelete = CombineRanges(toDelete, target.Cells(currentRow, 1))
        End If
    Next
    
    If Not toDelete Is Nothing Then toDelete.Delete xlUp
    
End Sub

Notice:

• Consistent indentation makes it easier to identify where code blocks begin and end.
• Meaningful variable names make it easier to understand what's going on and why.
• Variables being declared closer to their usage (as opposed to "at the top of the procedure") makes reading the code more fluid.
• Working with object references instead of against the current Selection makes the code much more straightforward.
• Variables meant to hold a row number are declared as Long, because a 64-bit worksheet can have [many] more than 65,535 rows.
• Procedure names are PascalCase; avoid using underscores in procedure names, VBA uses them for private Object_EventName handlers and Interface_Member implementation procedures.