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.
