3
\$\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
\$\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\$
    – PeterT
    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?

\$\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\$
    – Greedo
    Apr 11, 2020 at 8:31

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.