Removing the word “NULL” from a single column in Excel using VBA

I am using VBA in Excel to remove the word NULL from cells. I have the following code:

Sub RemoveNull()
Dim c, count, r
Application.ScreenUpdating = False

count = 0
r = ActiveCell.row
c = ActiveCell.Column
c = GetLetterFromNumber(c)

For x = 1 To Range(c & Rows.count).End(xlUp).row Step 1
count = count + 1
If Range(c & x).Value = "NULL" Then
Range(c & x).Clear
End If
'This section is for trouble shooting
'If count = 1000 Then
'    Range(c & x).Select
'    count = 1
'End If
Next
Application.ScreenUpdating = True
MsgBox "Finished"
Range(c & r).Select
End Sub


This works just fine and is designed to only check one column. I don't want to check the whole sheet with this I already have another sub that does this.

I want to make sure that this is the most efficient way to go about it. right now it checks each cell from top to bottom and then clears the contents if it contains NULL.

Static Code Analysis

I took your code and ran it through Rubberduck (an open-source VBIDE add-in project I manage) code inspections - here's what came out of it:

• Implicit references to ActiveSheet

When they're not qualified with an actual Worksheet object instance, members Range and Rows (and Columns, Names and Cells) implicitly refer to the ActiveSheet. These implicit references make the code frail, and harder to debug. Consider making these references explicit when they're intended (e.g. ActiveSheet.Range instead of just Range; ActiveSheet.Rows instead of just Rows), and prefer working off object references (e.g. DataSheet.Range, DataSheet.Rows).

• Procedure RemoveNull is implicitly Public

Module members are public by default, which can be counter-intuitive (especially if you're coming from other programming languages). Consider specifying explicit access modifiers to avoid ambiguity. e.g. Public Sub RemoveNull() instead of just Sub RemoveNull().

• Instruction contains multiple declarations

Dim c, count, r


Declaring multiple variables in the same instruction is legal, but should be used sparingly. Consider declaring variables closer to their usage, in a single instruction per declaration.

• Implicit Variant variables

Variables c, r, and count are all implicitly Variant, because they're declared without an As clause. Consider specifying an appropriate data type for all variables (e.g. String for strings, Long for row numbers, etc.), and use an explicit Variant where Variant is intended. Note that there's a common beginner trap here:

Dim c, count, r As Long ' only r is a Long, c and count are both Variant

• Use meaningful names

c, r, and x are poor names. Prefer using names that convey usage and meaning, e.g. activeColumn, activeRow, and currentRow. Avoid disemvoweling, numeric suffixes, Systems Hungarian prefixes and 1-2 character identifiers.

• Undeclared variables

Variable x isn't declared, which means it's an implicit Variant, declared on-the-fly by the VBA runtime. If your code can compile and run, it means Option Explicit isn't specified either.

• Option Explicit not specified

VBA will happily compile a typo: use Option Explicit to prevent successfully compiling a program that doesn't explicitly declare every single variable it's using.

Future versions of Rubberduck will also highlight that Step 1 is the default step for a For loop, and therefore redundant.

Redundancies

Static code analysis is nice, but can't pick up everything. Like this:

c = ActiveCell.Column
c = GetLetterFromNumber(c)


You have the Column number already - there's absolutely no need to work out what column "letter" that number is referring to. Simply change how you're accessing the cell - instead of this:

[ActiveSheet.]Range(c & x).Clear


Use Cells:

[ActiveSheet.]Cells(x, c).Clear


Notice how much easier that is to parse, with meaningful names:

[ActiveSheet.]Cells(currentRow, activeColumn).Clear


So, remove GetLetterFromNumber, you don't need it at all. Ever.

Static code analysis also won't pick up (for now, anyway) that Range.Value has a non-zero possibility of containing an error value - in which case a comparison with any non-error type will raise run-time 13 / Type Mismatch:

If Range(c & x).Value = "NULL" Then ' possible error here


The loop body is pulling the same cell twice, and in order to validate whether it's an error or not, you'd need to pull it a third time: consider extracting a local variable.

Dim currentCell As Range
Set currentCell = [ActiveSheet.]Cells(x, c)

If Not IsError(currentCell.Value) Then
If currentCell.Value = "NULL" Then ' comparison is now safe
currentCell.Clear
End If
End If


The last thing you want is the possibility of throwing a run-time error *while Application.ScreenUpdating is turned off. For this reason, whenever you need to toggle that setting, consider introducing some minimal error handling:

Public Sub DoSomething()
On Error GoTo CleanFail
Application.ScreenUpdating = False

'code...

CleanExit:
Application.ScreenUpdating = True
Exit Sub
CleanFail:
Debug.Print Err.Description
Stop ' we don't know what happened; break here and debug
Resume CleanExit
End Sub


Performance

Range is an object collection - the fastest way (by a lot) to iterate it is with a For Each loop. But, if all you need out of a Range its its Value, you can iterate the values even faster by dumping the range into an array, and iterating the array with a For...Next loop:

With ActiveSheet

Dim lastRow As Long
lastRow = .Cells(.Rows.count, c).End(xlUp).Row

Dim values As Variant ' a variant array
values = .Range(.Cells(1, 1), .Cells(.Rows.Count, lastRow)).Value

End With


There are plenty of Stack Overflow Q&A's about the best ways of turning that 2D array into a 1D array, I'll leave that bit of research up to you.

Once you have a one-dimensional array, you can iterate and modify it in-memory:

For x = LBound(values) To UBound(values)
If Not IsError(values(x)) Then
If values(x) = "NULL" Then values(x) = vbEmpty
End If
Next


And then you can convert back the 1D array to a 2D array, and dump it onto the worksheet in one single write:

ActiveSheet.Cells(1, activeColumn).Resize(UBound(values)) = Application.Transpose(values)


And with one single read and one single write from the worksheet, and the iteration and actual work being done in-memory at lightning speed (as opposed to one cell at a time with Excel repainting itself every time, if not recalculating the whole worksheet too), there shouldn't even be a need to switch off Application.ScreenUpdating now - toggling it off won't make a difference anyway.

I suspect the result will be so fast, you won't even need that MsgBox call to wake the user up.

• Thank you for the thoroughness of your response. I have implemented most of your suggestions and as stated it works almost instantly even for 65k+ rows. I do have a follow-up question though. What is the purpose or best practice for diming the variables in different places instead all at once in a block at the top or even in one line? – Mike Sep 26 '17 at 18:52
• Code should be written to be read, not just executed. Wall-of-declarations style is often brought up to defend [Systems] Hungarian Notation - "that way I know the type of the variables where I use them", vs. declare them where you use them, and use meaningful names; no need for a "type" prefix when the declared type is in your face. Also reduces cognitive load (especially with long procedures) and pretty much eliminates the possibility for declaring a variable that you're not using. – Mathieu Guindon Sep 26 '17 at 18:58
• Basically, for short procedures it probably doesn't matter. For everything else, a wall-of-declarations is a maintenance burden. – Mathieu Guindon Sep 26 '17 at 19:01
• Makes perfect sense, though I've never thought about it. Again thanks for the help it is much appreciated! – Mike Sep 26 '17 at 19:24
• @Mike no problem! And give Rubberduck a star on GitHub if you like! =) – Mathieu Guindon Sep 26 '17 at 19:25

Regarding OP - everything Mat said

I think Range.Replace is more efficient in this case - faster (and less code)

Option Explicit

Public Sub RemoveNull()
With ThisWorkbook.ActiveSheet.UsedRange
With .Columns(ActiveCell.Column - .Column + 1)
.Replace What:="NULL", Replacement:=vbNullString, LookAt:=xlWhole, MatchCase:=False
End With
End With
End Sub