4
\$\begingroup\$

I have a function that essentially searches out a value in a table by looking through the column headers and row headers to locate a data point. The function takes the sheet name, the column header name, and row header name as arguments, and I have an error handler set up if any one of the methods performed with those variables fails. However, each handler is very similar, so there is much repetition. Is there a more clear way to have all errors handled uniquely but not to have to re-write Exit Function and other such things?

Function getDataPoint(rowItem As String, searchCol As Integer, colItem As String, searchRow As Integer, shtName As String) As Variant

    Dim rowNum As Integer, colNum As Integer
    Dim rowNumRng As Range, colNumRng As Range

    'Find worksheet to search
    On Error GoTo shtNotFound:
    With ThisWorkbook.Sheets(shtName)

        'Find rowNum where rowItem appears in searchCol
        On Error GoTo rowNotFound:
        Set rowNumRng = .Columns(searchCol).Find(What:=rowItem, _
            After:=.Cells(1, searchCol), _
            LookIn:=xlValues, _
            LookAt:=xlWhole, _
            SearchOrder:=xlByColumns, _
            SearchDirection:=xlNext, _
            MatchCase:=True)

        rowNum = rowNumRng.Row

        'Find colNum where colItem appears in searchRow
        On Error GoTo colNotFound:
        Set colNumRng = .Rows(searchRow).Find(What:=colItem, _
            After:=.Cells(searchRow, 1), _
            LookIn:=xlValues, _
            LookAt:=xlWhole, _
            SearchOrder:=xlByRows, _
            SearchDirection:=xlNext, _
            MatchCase:=True)

        colNum = colNumRng.Column

        getDataPoint = .Cells(rowNum, colNum).Value

    End With
    On Error GoTo 0
    Exit Function

 shtNotFound:
    Debug.Print "Sheet not found"
    getDataPoint = "NOT FOUND"
    Exit Function
 rowNotFound:
    Debug.Print "Row item not found"
    getDataPoint = "NOT FOUND"
    Exit Function
 colNotFound:
    Debug.Print "Column item not found"
    getDataPoint = "NOT FOUND"
    Exit Function

End Function
\$\endgroup\$
0

3 Answers 3

4
\$\begingroup\$

If you absolutely need to use your Find approach, or if you'd just like to see a centralized error handler, as per you question, then you can use a variable to store the custom error message, and then refer to that variable from the error handler:

Also, in you On Error statement, you don't need the trailing ":" as that indicates that the line will have another statement following the ":"

Function getDataPoint(rowItem As String, searchCol As Integer, colItem As String, searchRow As Integer, shtName As String) As Variant

  Dim rowNum As Integer, colNum As Integer
  Dim rowNumRng As Range, colNumRng As Range
  Dim sError As String

  'Find worksheet to search
  sError = "Sheet not found"
  On Error GoTo itmNotFound
  With ThisWorkbook.Sheets(shtName)
    'Find rowNum where rowItem appears in searchCol
    sError = "Row not found"
    Set rowNumRng = .Columns(searchCol).Find(What:=rowItem, _ After:=.Cells(1, searchCol), _ LookIn:=xlValues, _ LookAt:=xlWhole, _ SearchOrder:=xlByColumns, _ SearchDirection:=xlNext, _ MatchCase:=True)
    rowNum = rowNumRng.Row
    'Find colNum where colItem appears in searchRow
    sError = "Column not found"
    Set colNumRng = .Rows(searchRow).Find(What:=colItem, _ After:=.Cells(searchRow, 1), _ LookIn:=xlValues, _ LookAt:=xlWhole, _ SearchOrder:=xlByRows, _ SearchDirection:=xlNext, _ MatchCase:=True)
    colNum = colNumRng.Column
    getDataPoint = .Cells(rowNum, colNum).Value
  End With
  On Error GoTo 0
  Exit Function

itmNotFound:
  Debug.Print sError
  getDataPoint = "NOT FOUND"

End Function
\$\endgroup\$
3
  • \$\begingroup\$ This won't ever set sError if I'm reading this correctly. \$\endgroup\$
    – RubberDuck
    Commented Mar 17, 2016 at 9:14
  • \$\begingroup\$ hmm, other than Markdown mangling the line breaks in the Find methods, the code is good, and sError is set and printed when the row, column or sheet is not found. sError is a prescriptive string, so it's being set before the error occurs. If no error occurs, sError is never set, and never debug.printed. \$\endgroup\$ Commented Mar 17, 2016 at 10:14
  • \$\begingroup\$ Ahh ha! I see now. My bad. \$\endgroup\$
    – RubberDuck
    Commented Mar 17, 2016 at 10:21
3
\$\begingroup\$

Find is a very slow method for doing what you need.

You will achieve much greater performance, and avoid the need for complex error handling, by reading the search range into an array, and looping over the first dimension to find the row, then looping over the second to find the column.

Edit: Depending upon the number of rows and columns that you expect, or perhaps even after inspecting the size of the search array, you could squeeze out some extra performance by checking to see if there are fewer columns than rows and then check if the column exists, before checking if the row exists. And if there are fewer rows than columns, then check to see if the row exists before checking if the column exists.

Edit2: if you'll be calling this function multiple times, with similar sheet names, row values or column values, you could optimize further by keeping Scripting.Dictionarys of column names and indexes, and row names and indexes. That would be the fastest approach.

\$\endgroup\$
5
  • \$\begingroup\$ Thanks for the tips. Also, what would be the advantage to using Scripting.Dictionary rather than a collection? \$\endgroup\$
    – teepee
    Commented Mar 16, 2016 at 21:41
  • \$\begingroup\$ Scripting.dictionary has an Exists method and uses a hash table behind the scenes, so it will be faster at determining whether a column or row exists, before you try to index it. \$\endgroup\$ Commented Mar 16, 2016 at 21:46
  • \$\begingroup\$ OK that's good to know, I'll give that a shot. But I'm wondering still about the error handling aspects, any thoughts? \$\endgroup\$
    – teepee
    Commented Mar 16, 2016 at 21:48
  • \$\begingroup\$ See my other answer \$\endgroup\$ Commented Mar 16, 2016 at 21:56
  • \$\begingroup\$ So I have posted an answer based on your advice about creating an array to search. This appears to be noticeably faster than using find (7e-3 sec. rather than 1.5e-2 sec.). Is this what you had in mind? \$\endgroup\$
    – teepee
    Commented Mar 17, 2016 at 21:51
2
\$\begingroup\$

First, your arguments are being passed ByRef by default. Try to pass them ByVal if possible -

Public Function getDataPoint(ByVal rowItem As String, ByVal searchCol As Long, ByVal colItem As String, ByVal searchRow As Long, ByVal shtName As String) As Variant

You'll also notice I changed the interger items to long. Integers - integers are obsolete. According to msdn VBA silently converts all integers to long. I'd do the same for the other variables.

Speaking of variables - Variable names - give your variables meaningful names. Right now it's difficult to differentiate searchCol and colNum and colItem. Why not searchColumn, foundColumn and findColumnString or something along those lines so you don't need to go back to reference which is which.

Public Function getDataPoint(ByVal searchRowString As String, ByVal searchColumn As Long, ByVal searchColumnString As String, ByVal searchRow As Long, ByVal sheetName As String) As Variant

    Dim foundRow As Long
    Dim foundColumn As Long
    Dim foundRowRange As Range
    Dim foundColumnRange As Range

First, no need to handle the worksheet as a goto error when you can just use something like this -

If Not Evaluate("ISREF('" & shtName & "'!A1)") Then
    Debug.Print "Sheet not found"
    GoTo NotFound
End If

NotFound:
    getDataPoint = "NOT FOUND"
End Sub

Now you can send all errors to one label and just debug.print when the error occurs. Or as suggested above - carry the debug.print as a string, which is clever.

To speed up search I'd take a look at this answer on stackoverflow. Also, a good way to speed up your macro is to use Application.Screenupdating = False and Application.Calculation = xlManual and Application.EnableEvents = False. Just be sure to return them to True and xlAutomatic and True before exiting the sub.


One other idea I had was to use a vlookup and hlookup to get the row and column numbers to avoid a find. Then put them together for the data point.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.