Count rows of a table

Question

I am trying to count the rows of two tables generated by SQL connections in Excel VBA. My plan was to use ListObject.DataBodyRange.Rows.Count however, this errors when a table is empty, which one often is. I found it completely impossible to use On Error GoTo ... twice within one sub so I pulled out the counts as functions like this. I based my error handling pattern on this answer. Is this the best way to go?

Function CountCurrencyTrades() As Integer
 Dim sheetCurrencyTable As Worksheet
 Set sheetCurrencyTable = Worksheets("CurrencyTable")
 Dim tblCurrencyTrades As ListObject
 Set tblCurrencyTrades = sheetCurrencyTable.ListObjects("CurrencyQuery")
 On Error GoTo CurrencyTradesEmpty
 CountCurrencyTrades = tblCurrencyTrades.DataBodyRange.Rows.Count
CleanExit:
 Exit Function
CurrencyTradesEmpty:
 CountCurrencyTrades = 0
 Resume CleanExit
End Function

Answer 1

Just some things that jump out at me:

---

You don't reset OnError

Huge red flag right there. If you're going to change OnError Goto you should immediately write the corresponding lines to set it back to normal.

Specifically: (Square brackets for emphasis)

 On Error GoTo CurrencyTradesEmpty
 CountCurrencyTrades = tblCurrencyTrades.DataBodyRange.Rows.Count
 [On Error Goto 0] 
CleanExit:
 Exit Function
CurrencyTradesEmpty:
 [On Error Goto 0] 
 CountCurrencyTrades = 0
 Resume CleanExit

---

Your function has too many responsibilities

Single Responsibility Principle. Your function has a hardcoded worksheet, hardcoded listObject and then handles the counting of said object. I would have the function take a ListObject as an argument and let another function handle the retrieval of said objects. Like so:

 Option Explicit
 Public Function GetCurrencyTradesCount() As Long
 Dim sheetCurrencyTable As Worksheet
 Set sheetCurrencyTable = Worksheets("CurrencyTable")
 Dim tblCurrencyTrades As ListObject
 Set tblCurrencyTrades = sheetCurrencyTable.ListObjects("CurrencyQuery")
 GetCurrencyTradesCount = CountListRows(tblCurrencyTrades)
 End Function
 Private Function CountListRows(ByRef targetList As ListObject) As Long
 '/ On Error (I.E. empty list), returns 0
 On Error GoTo CleanFail
 CountListRows = targetList.DataBodyRange.Rows.Count
 On Error GoTo 0
CleanExit:
 Exit Function
CleanFail:
 On Error GoTo 0
 CountListRows = 0
 Resume CleanExit
 End Function

Scoping

Your function is implicitly public. You should explicitly set a Public or Private scope for it.

---

Integer

Is officially deprecated. The compiler will silently convert all Integers to Longs, so just use Long instead.

---

Error Handling

Do you really want your function to return 0 for all errors? What if it's given an empty object reference? I would rename your label to CleanFail: and think about what other failure scenarios it might want to deal with.

• Dan: Can you expand on why I need the On Error GoTo 0 and what happens if I don't have it? I agree about passing the ListObject.
• Kaz: When you change On Error, your change remains in effect until you change it again. Just imagine, for a second, what happens if the first line in CleanFail: causes an error? It would immediately jump back to the label, error again and create an infinite loop.
• Kaz: On Error Goto 0 clears all manually-set error handling and resets to standard. So, as soon as you have executed the statement that your error handling was intended for, you should either change the error handling or reset it to normal.
• Dan: Will it also reset when the function ends? i.e. is the manually-set error handling scoped to the function it is set in or is it global?
• Kaz: All Err objects (whose properties On Error modifies) are locally scoped within a procedure. So any On Error changes will only apply to the procedure they're in. N.B. though, if a sub procedure causes an error with no handling, VBA will trace its way back up the call stack until it finds an error handler. For a more complete overview, here is the MSDN page on handling Run-Time Errors

Answer 2

You can avoid the extra goto, and use inline error resetting, like this:

Function CountCurrencyTrades() As Integer
 Dim sheetCurrencyTable As Worksheet
 Set sheetCurrencyTable = Worksheets("CurrencyTable")
 Dim tblCurrencyTrades As ListObject
 Set tblCurrencyTrades = sheetCurrencyTable.ListObjects("CurrencyQuery")
 CountCurrencyTrades = 0
 On Error Resume Next
 CountCurrencyTrades = tblCurrencyTrades.DataBodyRange.Rows.Count 
 On Error GoTo 0
End Function