patternMinor
Excel SLOOKUP UDF
Viewed 0 times
slookupexceludf
Problem
I created an Excel AddIn that has a function called
```
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
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
Solution
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 ThenNote 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.Code Snippets
If TypeOf caller Is Excel.Range ThenContext
StackExchange Code Review Q#101074, answer score: 4
Revisions (0)
No revisions yet.