patternModerate
VBA Function slower than Excel Formula
Viewed 0 times
excelthanslowerfunctionvbaformula
Problem
I've replaced the following horrendous Excel formula (values and names have been replaced):
With a VBA Function so it would be easier to read and supposedly faster:
I get the same result as the function however it does it two times longer than the formula when working with 2k rows. Why isn't the code faster than the Excel formula? Can it be improved? The file is expected to grow so it means the code will be of no use.
Edit: Using more arguments improved the macro, see my response below.
=IF(AND(SourceTable[@Field1]="Value1",SourceTable[@Field2]="Value2"),"Result1",
IF(SourceTable[@Field2]="Value3","Result2",
IF(AND(SourceTable[@Field1]="Value4",SourceTable[@Field3]="Value5"),"Result3",
IF(AND(SourceTable[@Field1]="Value6",OR(SourceTable[@Field3]="Value7",SourceTable[@Field3]="Value8")),"Value9",
VLOOKUP(SourceTable[@Field1];MatchingTable;2;0)))))With a VBA Function so it would be easier to read and supposedly faster:
Function CUSTOMFUNCTION(oRngField1 As Range, oRngField2 As Range, oRngField3 As Range) As String
'If imbrication
If (oRngField1 = "Value1" And oRngField2 = "Value2") Then
CUSTOMFUNCTION = "Result1"
Exit Function
ElseIf oRngField2 = "Value3" Then
CUSTOMFUNCTION = "Result2"
Exit Function
ElseIf oRngField1 = "Value4" Then
If oRngField3 = "Value5" Then
CUSTOMFUNCTION = "Result3"
Exit Function
ElseIf (oRngField3 = "Value6" Or oRngField3 = "Value7") Then
CUSTOMFUNCTION = "Result4"
Exit Function
End If
End If
'Vlookup if no match before
Dim sTableMatchingFieldLookup As String
Dim sTableMatchingFieldResult As String
sTableMatchingFieldLookup = "MatchingTable[Field1]"
sTableMatchingFieldResult = "MatchingTable[Field2]"
CUSTOMFUNCTION = Range(sTableMatchingFieldResult)(Application.Match(oRngField1, Range(sTableMatchingFieldLookup), 0))
End FunctionI get the same result as the function however it does it two times longer than the formula when working with 2k rows. Why isn't the code faster than the Excel formula? Can it be improved? The file is expected to grow so it means the code will be of no use.
Edit: Using more arguments improved the macro, see my response below.
Solution
I'm assuming that names have been changed to protect the innocent, but just in case they've not been...
These are terrible names. All of them.
-
Parameters should be
should be passed in and NEVER should they be numbered the way
this is. So, maybe this instead (again, I have to guess because I
have no clue):
-
Don't use hungarian notation. Just... please don't. It should be
killed with fire and buried out back.
Other Stuff
constant and use that constant anywhere they show up. (Of course,
use meaningful names for them.)
Things you did right
These are terrible names. All of them.
- Function names should have verb-noun names and be
PascalCased. So instead ofCUSTOMFUNCTIONshould beGetCustomMatch(or something like that. I can't say because you've ommitted any actual names...)
-
Parameters should be
camelCase and tell me what kind of datashould be passed in and NEVER should they be numbered the way
this is. So, maybe this instead (again, I have to guess because I
have no clue):
Function GetCustomMatch(LookupRange As Range, lastYearsSalesRange as Range, iJustDontKnowWhatThisIsRange as Range) As String-
Don't use hungarian notation. Just... please don't. It should be
killed with fire and buried out back.
Other Stuff
- Some of your "Values" are used in multiple places. Store them in a
constant and use that constant anywhere they show up. (Of course,
use meaningful names for them.)
- I have a feeling that
sTableMatchingFieldLookupand...Resultshould be actual ranges instead of strings. String typing is bad. If you insist on keeping them as strings, store the values as constants.
- You should be able to pick up some performance by setting
Application.Calculation = xlCalculationManual. This Stack Overflow answer has a pretty good example of what you're trying to do.
Things you did right
- You exit early if you've already found a match before performing the lookup rather than nesting
ifstatements.
Code Snippets
Function GetCustomMatch(LookupRange As Range, lastYearsSalesRange as Range, iJustDontKnowWhatThisIsRange as Range) As StringContext
StackExchange Code Review Q#56023, answer score: 14
Revisions (0)
No revisions yet.