patternMinor
Function to find the kth match
Viewed 0 times
findthematchfunctionkth
Problem
I've created a UDF some time ago and used it pretty often till now in different ways (but mainly to compare a "history" of data like having different filters at the same time). While there is no need to change anything at this state, I'd like get some suggestions of how to improve it without the loss of any functionality.
To make it short, it is a LOOKUP-function which goes for the xth match and returns a ref.
Normally there are comments, but I deleted them to test if it is still easy to get what it does. Also i like to have it in a way i don't need them.
```
Public Function VLOOKUPK(lookup_value As String, lookup_vector As Range, Optional result_vector As Range, _
Optional count_num As Long = 1, Optional case_sens As Boolean) As Variant
Application.Volatile
Dim i(0 To 1) As Long, j As Long, lookup_vector_values As Variant
VLOOKUPK = CVErr(2023)
If count_num - 1 <> Abs(Round(count_num - 1)) Or (i(0) * i(1)) = 1 Then Exit Function
Set lookup_vector = Intersect(lookup_vector.Parent.UsedRange, lookup_vector)
If lookup_vector.Areas.Count > 1 Then Set lookup_vector = lookup_vector.Areas(1)
i(0) = lookup_vector.Rows.Count
i(1) = lookup_vector.Columns.Count
If i(0) > 2 And i(1) > 2 Then Exit Function
If result_vector Is Nothing Then
If i(0) = 1 Or i(1) = 1 Then
Set result_vector = lookup_vector
ElseIf i(0) > 2 And i(1) = 2 Then
Set result_vector = lookup_vector.Columns(2)
Set lookup_vector = lookup_vector.Columns(1)
ElseIf i(0) = 2 And i(1) > 2 Then
Set result_vector = lookup_vector.Rows(2)
Set lookup_vector = lookup_vector.Rows(1)
Else
Exit Function
End If
Else
If result_vector.Columns.Count > 1 And result_vector.Rows.Count > 1 Then Exit Function
Set result_vector = Intersect(result_vector.Parent.UsedRange, result_vector)
End If
If lookup_vector.Columns.Count > 1 And lookup_vector.Rows.Count > 1 Then Exit Function
If Not case_sens The
To make it short, it is a LOOKUP-function which goes for the xth match and returns a ref.
Normally there are comments, but I deleted them to test if it is still easy to get what it does. Also i like to have it in a way i don't need them.
```
Public Function VLOOKUPK(lookup_value As String, lookup_vector As Range, Optional result_vector As Range, _
Optional count_num As Long = 1, Optional case_sens As Boolean) As Variant
Application.Volatile
Dim i(0 To 1) As Long, j As Long, lookup_vector_values As Variant
VLOOKUPK = CVErr(2023)
If count_num - 1 <> Abs(Round(count_num - 1)) Or (i(0) * i(1)) = 1 Then Exit Function
Set lookup_vector = Intersect(lookup_vector.Parent.UsedRange, lookup_vector)
If lookup_vector.Areas.Count > 1 Then Set lookup_vector = lookup_vector.Areas(1)
i(0) = lookup_vector.Rows.Count
i(1) = lookup_vector.Columns.Count
If i(0) > 2 And i(1) > 2 Then Exit Function
If result_vector Is Nothing Then
If i(0) = 1 Or i(1) = 1 Then
Set result_vector = lookup_vector
ElseIf i(0) > 2 And i(1) = 2 Then
Set result_vector = lookup_vector.Columns(2)
Set lookup_vector = lookup_vector.Columns(1)
ElseIf i(0) = 2 And i(1) > 2 Then
Set result_vector = lookup_vector.Rows(2)
Set lookup_vector = lookup_vector.Rows(1)
Else
Exit Function
End If
Else
If result_vector.Columns.Count > 1 And result_vector.Rows.Count > 1 Then Exit Function
Set result_vector = Intersect(result_vector.Parent.UsedRange, result_vector)
End If
If lookup_vector.Columns.Count > 1 And lookup_vector.Rows.Count > 1 Then Exit Function
If Not case_sens The
Solution
Volatile
I'm walking down the code as I'm writing this, so I haven't really seen the code below that line. The Application.Volatile method call tells Excel to recalculate your function every time any cell anywhere on the worksheet is modified. The default behavior is to recalculate only when the input variables change.
That method call on the
A comment explaining why
Naming notes...
I like the consistency you have with your naming style, it's nice to see. The style matches that of native Excel worksheet functions, like
SUMIF(range, criteria, [sum_range])
I think it's fine to expose that style to outside code (and/or Excel), but I don't see a reason for the internals to use any
Hmm. 3 variables declared in the same instruction, of 3 different types and meanings. For readability's and maintainability's sake, a few points:
I'd get rid of
I said I was going to get back to the loop:
Why make that operation a side-effect of iterating the loop? It's non-standard; people usually use a variable in a
Application.VolatileI'm walking down the code as I'm writing this, so I haven't really seen the code below that line. The Application.Volatile method call tells Excel to recalculate your function every time any cell anywhere on the worksheet is modified. The default behavior is to recalculate only when the input variables change.
That method call on the
Application object tells me that you've perhaps tested it without at first, and concluded that it was required in order for the function to properly recalculate.A comment explaining why
Application.Volatile is needed here, might be judicious.Naming notes...
I like the consistency you have with your naming style, it's nice to see. The style matches that of native Excel worksheet functions, like
SUM:SUMIF(range, criteria, [sum_range])
I think it's fine to expose that style to outside code (and/or Excel), but I don't see a reason for the internals to use any
snake_case, convention is to use camelCase for locals (and usually, parameters too), and PascalCase for member names (procedures, functions, properties, ..).Dim i(0 To 1) As Long, j As Long, lookup_vector_values As VariantHmm. 3 variables declared in the same instruction, of 3 different types and meanings. For readability's and maintainability's sake, a few points:
- Declare arrays on their own line.
As Long, j As Longis pretty much hiding the identifier.
- If you're going to declare more than one variable in one instruction, do so but with variables of the same type.
- Use meaningful identifier names:
iis usually used in aFor..Nextloop, to count iterations. An array of Int32's isn't exactly trivially that. We don't know anything about that variable until we stumble on a usage of it, which hopefully isn't too far. The first usage is actually 2 lines below, which isn't too bad, but... let's face it, we all hate it when it's down toiandland1.
jis also typically used for iteratingFoo..Nextloops, usually wheniis already taken. I'll have to scroll further down to see where the loop is. An interesting loop, I'll get back to it.
- Declare variables as close as possible to their first usage.
I'd get rid of
i, and rename lookup_vector to lookup_range or, as VLOOKUP has it, table_array (although, ...)I said I was going to get back to the loop:
For i(j) = 1 To UBound(lookup_vector_values, j + 1)Why make that operation a side-effect of iterating the loop? It's non-standard; people usually use a variable in a
For loop, not an expression - one must read it carefully. If it can't be refactored, then it should be commented.Code Snippets
Application.VolatileDim i(0 To 1) As Long, j As Long, lookup_vector_values As VariantFor i(j) = 1 To UBound(lookup_vector_values, j + 1)Context
StackExchange Code Review Q#114425, answer score: 4
Revisions (0)
No revisions yet.