5
\$\begingroup\$

I created an Excel AddIn that has a function called SLOOKUP which can replace INDEX MATCH combinations in an easy and smart way. For more info please visit my blog post. I would like you to optimize this further if possible.

Function SLOOKUP(database As Range, lookupCol As Range)
' Description:  Excel UDF that implements INDEX+MATCH combination in a smart way
'               by picking up [lookup_value] and [returnCol] relative to the [lookupCol]
' Author     :  Hussain Khamis
' Website    :  XpertInTech.com
' Version    :  0.8
'
' Definitions:
'
' INPUT:
' database As Range       :  The database to perform the lookups against; must include the headers as well
' lookupCol As Range      :  The name of the column that should contains the [lookup_value]
'
' INTERNAL:
' o_Caller As Object      :  Stores Application.Caller to determine later the type of the caller of this function
' r_Caller As Range       :  Sets r_Caller to the range in o_Caller after checking that o_Caller type is a range
' lookup_value As Variant :  The value, that would ideally be unique (like an ID), to retrieve the other columns based on
' returnCol As Variant    :  The name of the column that the returned value will come from
' i_lookupCol As Integer  :  The column number that will be looked up in the database and should contains the [lookup_value]
' i_returnCol As Integer  :  The column number that the final value will be retrieved from
' i_returnRow As Integer  :  The row number that intersects with i_returnCol which the final value will be retrieved from
'
On Error GoTo PROCErr

'Get lookup and returnCol relative to lookupCol
'Caller of this function must be one cell at a time or else the result will be an error
'Application.Volatile

Dim o_Caller As Object
Set o_Caller = Application.Caller

Select Case TypeName(o_Caller)
    Case "Range"
        Dim r_Caller As Range
        Set r_Caller = o_Caller
        If r_Caller.Count = 1 Then
            Dim lookup_value As Variant
            Dim returnCol As Variant
            With lookupCol
                lookup_value = .Offset(r_Caller.Row - .Row, 0)
                returnCol = .Offset(0, r_Caller.Column - .Column)
            End With
        Else
            GoTo PROCErr
        End If
    Case Else
        GoTo PROCErr
End Select

'Get lookupCol #
Dim i_lookupCol As Integer
i_lookupCol = WorksheetFunction.Match(lookupCol.Value2, database.Rows(1), 0)

'Get returnCol #
Dim i_returnCol As Integer
i_returnCol = WorksheetFunction.Match(returnCol, database.Rows(1), 0)

'Get returnRow #
Dim i_returnRow As Integer
i_returnRow = WorksheetFunction.Match(lookup_value, database.Columns(i_lookupCol), 0)

'Result
SLOOKUP = WorksheetFunction.Index(database, i_returnRow, i_returnCol)
Exit Function

'On error
PROCErr:
SLOOKUP = CVErr(xlErrNA)

End Function
\$\endgroup\$

migrated from stackoverflow.com Aug 16 '15 at 1:25

This question came from our site for professional and enthusiast programmers.

4
\$\begingroup\$

TypeName isn't a great way to check argument types. It literally checks the name and could produce unexpected results in certain situations. I would recommend using TypeOf instead. The Case statement also seems superfluous to me. There's only one branch, so an If statement does the job just fine.

If TypeOf caller Is Excel.Range Then

Note that I fully quantified the type for reasons pointed out in the answer I linked to.

I'm not a huge fan of directly jumping to line labels with GoTo, but I see why you did it. However, it's more semantically correct to raise an error instead. Being performance is a concern though, I would definitely benchmark the change vs. your current code. GoTo isn't entirely evil and this is a good use of it in my opinion.

You access database.Rows(1) a couple of times, you might save a (minuscule) amount of overheard by accessing it once and storing it in a variable instead. The gain won't be much, but could potentially add up if you use this UDF in a large number of cells. Again, benchmark it.

\$\endgroup\$
3
\$\begingroup\$

It took me quite a while to figure out what the function is doing - you might want to add a description to the top of the code to describe the function. Maybe something like -

'This function reconstructs a referenced table by matching a primary key to the table
'and returning information based on the structure of the table that's being 
'reconstructed. The formula utilizes its location to find and match the 
'adjacent primary key and return the information for its own specified header.

The definitions section in the code doesn't really translate well in to what the code does - it more gives the structure of how you're using the variables. Even when referencing back to the definitions while reading through the code, the definitions are still unclear.

You might want to comment some of the code as well to explain why certain things are happening-

Dim o_Caller As Object
Set o_Caller = Application.Caller

Select Case TypeName(o_Caller)
    'The application caller's location will define what attributes should be looked up in the "database" - so we reference off of that cell for the process.
    Case "Range"
        Dim r_Caller As Range
        Set r_Caller = o_Caller
        If r_Caller.Count = 1 Then
            Dim lookup_value As Variant
            Dim returnCol As Variant
            'The lookupCol contains the header for the table's primary key
            With lookupCol
                'The lookup_value is set to the table header that we want to return from the database. The returnCol is the column for the lookup_value
                lookup_value = .Offset(r_Caller.Row - .Row, 0)
                returnCol = .Offset(0, r_Caller.Column - .Column)
            End With
        Else
            GoTo PROCErr
        End If
    Case Else
        GoTo PROCErr
End Select

'Get lookupCol #
Dim i_lookupCol As Integer
i_lookupCol = WorksheetFunction.Match(lookupCol.Value2, database.Rows(1), 0)

'Get returnCol #
Dim i_returnCol As Integer
i_returnCol = WorksheetFunction.Match(returnCol, database.Rows(1), 0)

'Get returnRow #
Dim i_returnRow As Integer
i_returnRow = WorksheetFunction.Match(lookup_value, database.Columns(i_lookupCol), 0)

'Result
SLOOKUP = WorksheetFunction.Index(database, i_returnRow, i_returnCol)
Exit Function

'On error
PROCErr:
SLOOKUP = CVErr(xlErrNA)

End Function
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy