HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Eliminating empty spreadsheet cells from a range

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
rangeemptyeliminatingcellsspreadsheetfrom

Problem

I am copying data, that is one long list, into an excel sheet and I want to use a macro to organize it visually. The problem is that the data I am copying has empty cells that are unnecessary and will inhibit my code later in the macro and I want to get rid of them. This is what I came up with, but I am hoping there is a more efficient way to do it. The main problem I ran into in trying other methods is I couldn't set a value to a variable to end the second "For" statement before it tried to select a cell that doesn't exist. After it got to the first row it would keep running and would return an error on "ActiveCell.Offset(-1, 0).Range("A1").Select" because there was no cell to select.

Data example:

1, ,2, ,1, ,3, ,1, ,2, ,1, ,4, ,1, ,2, ,1, ,3, ,1, ,2, ,1,...

1, , ,1, , ,2, , ,1, , ,1, , ,2, , ,1, , ,1, , ,3, , ,1, ,...

Each comma represents a new cell, with the numbers and spaces (empty cells) in the same column.

My code:

Sub Organize_Data()

Dim n As Integer
Dim h As Integer
Dim t As Integer
h = 1000
t = h - 1

Cells(h, 1).Select
For n = 1 To t
    If IsEmpty(ActiveCell) Then
    ActiveCell.Offset(-1, 0).Range("A1").Select
    h = h - 1
    Else
    n = t
    End If
Next n

t = h - 1

For n = 1 To t
    If IsEmpty(ActiveCell) Then
    Selection.Delete Shift:=xlUp
    ActiveCell.Offset(-1, 0).Range("A1").Select
    h = h - 1
    Else
    ActiveCell.Offset(-1, 0).Range("A1").Select
    h = h - 1
    End If
Next n

End Sub


Data After Macro:

1,2,1,3,1,2,1,4,1,2,1,3,1,2,1,...

1,1,2,1,1,2,1,1,3,1,...

Possibilities:

I was thinking that in place of the first "For" statement I could use

Selection.End(xlUp).Select


but I don't know how I would set the variable "t" so that the "For" Statement would end before it tried to select a cell that didn't exist.
The other thing I can think of is maybe there is code that I could use for a third part to the "If" statement in the second "For" statement that will stop the "For" statement when it is in th

Solution

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.

Code Snippets

h = ActiveSheet.Range("A" & ActiveSheet.Rows.Count).End(xlUp).Row
'this loop sets n to the last row with data (right?)
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
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

Context

StackExchange Code Review Q#139551, answer score: 3

Revisions (0)

No revisions yet.