2
\$\begingroup\$

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
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Oct 15, 2017 at 3:03
\$\endgroup\$
1
  • 1
    \$\begingroup\$ I suspect that this portion of your code is not the problem. I can't see anything to fix that would make it run appreciably faster. Since it's a 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\$ Commented Oct 16, 2017 at 20:17

1 Answer 1

1
\$\begingroup\$

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?

answered Mar 22, 2018 at 4:38
\$\endgroup\$
1
  • \$\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\$ Commented Apr 11, 2020 at 8:31

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.