I wrote this code to interpolate precipitation data. When I run this code, it works very well, but the problem is that it slows my computer down so I have to wait 5 mins to interpolate the data.
Is there any way to make this function work faster?
Function IDWW(Value1, Value2, Value3, Dist1,Dist2,Dist3)
Dim a1 As Variant
Dim b1 As Variant
Dim a2 As Variant
Dim b2 As Variant
Dim a3 As Variant
Dim b3 As Variant
If Value1 <> "" And Dist1 <> "" Then
a1 = Value1 / (Dist1) ^ 2
b1 = 1 / (Dist1) ^ 2
Else
a1 = 0
b1 = 0
End If
If Value2 <> "" And Dist2 <> "" Then
a2 = Value2 / (Dist2) ^ 2
b2 = 1 / (Dist2) ^ 2
Else
a2 = 0
b2 = 0
End If
If Value3 <> "" And Dist3 <> "" Then
a3 = Value3 / (Dist3) ^ 2
b3 = 1 / (Dist3) ^ 2
Else
a3 = 0
b3 = 0
End If
'Avoid a problem if all 3 distances are empty
If b1 + b2 + b3 = 0 Then
IDWW = 0
Else
IDWW = (a1+a2+a3) / (b1+b2+b3)
End If
End Function
1 Answer 1
Naming
Your variable's names are pretty generic. Tell me what they are - use the name to give information - the characters are free.
The same goes for the function name - IDWW
. I'm not sure what it should be doing.
I imagine you'd be good with names like
firstValue, secondValue, thirdValue, firstDistance, secondDistance, thirdDistance
I don't know what those are because they aren't given a Type. Your function also doesn't have a return Type. That means you're taking in 6 variants and returning a variant. That has impact.
Variant
When you don't define or type your variable, VBA will declare it as a Variant, which are objects:
Performance. A variable you declare with the Object type is flexible enough to contain a reference to any object. However, when you invoke a method or property on such a variable, you always incur late binding (at run time). To force early binding (at compile time) and better performance, declare the variable with a specific class name, or cast it to the specific data type.
Total you are working with 12 Variants and a return Variant.
You also have 6 variables made of 1 letter and 1 number. The code is already pretty abstract, but seeing something like
For i = k To j Step -1
For n = 1 To 26
If k * n > j * n Then
n = i * j
Else
n = i * k
End If
Next
Next
Gives me no confidence that I'll be able to decipher what's happening.
You also test like this twice -
If Value2 <> "" And Dist2 <> "" Then
If Value3 <> "" And Dist2 <> "" Then
It seems like these should be optional arguments in the function, otherwise you're forcing the user to submit nulls.
Private Function IDWW(ByVal firstValue As Long, ByVal firstDistance As Long, _
Optional ByVal secondValue As Long, Optional ByVal secondDistance As Long, _
Optional ByVal thirdValue As Long, Optional ByVal thirdDistance As Long) As Long
You can also give them default values. If you don't give them defaults, they choose the default value of their Type.
Optional ByVal secondValue As Long = 0, Optional ByVal secondDistance As Long = 0,
You were testing for ""
which has a name = vbNullString
. But what you should have been testing for is the equivalent for a Variant
If IsEmpty(var) then
Something else you aren't testing for is whether the user passes something else in the Variant, like a Range. Or maybe a negative number. These are all things that can be eliminated with the right Type.
Like here -
If Value2 <> "" And Dist2 <> "" Then
a2 = Value2 / (Dist2) ^ 2
b2 = 1 / (Dist2) ^ 2
Else
a2 = 0
b2 = 0
End If
If a2
and a3
were Long
you wouldn't need to set them, they are by default 0
.
At the end
'Avoid a problem if all 3 distances are empty If b1 + b2 + b3 = 0 Then IDWW = 0
Is that really what you're doing? Adding variants to get 0. But what if I submitted a negative distance? What if I submitted both a negative value and distance?
-
\$\begingroup\$ I think this answer contains some really good points, especially with regard to naming and checking of arguments, which OP would do well to take on board. However I think there are a number of misleading statements too, which you might want to fix. For example, Variant can hold objects as you say, and if OP is working with ranges then late binding is a concern (not just for performance). However all the 2 letter variables store value types and having these in variants will have negligible impact on speed, instead the concern is type safety (as you touch upon). That object link is for VB.Net \$\endgroup\$Greedo– Greedo2020年04月11日 08:31:14 +00:00Commented Apr 11, 2020 at 8:31
Function
(and to make it more correct you should declare the return type), you're probably calling it using values straight from worksheet cells. If so, then this is where your speed problem lies. Make a block conversion of your worksheet cells to a memory array and then cell this function using array values. That will give you a tremendous speed boost. Overall though, we have to see more of your code to help. \$\endgroup\$