Please critique this UDF

JenniferMurphy

Well-known Member
Joined
Jul 23, 2011
Messages
1,447
Office Version
  1. 365
Platform
  1. Windows
I need to do some calculations (sum, average, count) on a collection of lists of integers that represent product ratings. The lists are of variable length. I have written a little UDF that will do one of several calculations on each list. Here's some sample data:

1608623777076.png


And here's the UDF code:

VBA Code:
Function ListCalc(pList, Optional pType As String = "Average")

Dim ListArray() As String
ListArray = Split(pList, " ")
Dim ListSum As Double
ListSum = 0
Dim ListCount As Long
ListCount = UBound(ListArray)
Dim I As Integer

For I = 0 To ListCount
    ListSum = ListSum + CDbl(ListArray(I))
Next I

Select Case UCase(pType)
    Case "SUM"
        ListCalc = ListSum
    Case "COUNT"
        ListCalc = ListCount + 1
    Case Else
        ListCalc = ListSum / (ListCount + 1)
End Select

End Function

Is there anything in this code that could be better? I never know, for example, if I am using the best datatypes.

Thanks
 

Excel Facts

Excel Wisdom
Using a mouse in Excel is the work equivalent of wearing a lanyard when you first get to college

eirikdaude

Board Regular
Joined
Nov 26, 2013
Messages
60
I guess you are already aware that there are already existing functions in Excel which will do what your UDF does?

Strictly speaking using Longs instead of Integers for all integers in Excel VBA is better, because they will be converted to longs anyway.

If you just want to get the count of the range passed to the function, there is no reason to loop through your array.

You don't specify the datatype of the plist argument of the function, or what datatype the function will return. I like to make these things explicit whenever I can. Do you run into some trouble with, for instance, the Split function if you specify pList as a range?

All that said, there I'll reiterate my first point - there are already functions for doing what you describe here in Excel, and I'd be absolutely shocked if it was possible to get even close to the same efficiency from a UDF as from a native function.

- edit - On reading your post a bit closer, I see that you have several numbers in each cell, which makes the approach you're taking more understandable. That will teach me to just skim the source data. I think you may still be better off just using worksheet-functions (sum, average, counta, etc.) on the array you end up with though. I believe all of these will take an array as their argument
 
Last edited:

footoo

Well-known Member
Joined
Sep 21, 2016
Messages
3,219
Office Version
  1. 2016
Platform
  1. Windows
eirikdaude
I think you're mis-reading the OP's requirement which is to Sum/Count/Average the numbers in each cell in column F.
The UDF looks OK to me.
 

FormR

MrExcel MVP
Joined
Aug 18, 2011
Messages
6,493
Office Version
  1. 365
Platform
  1. Windows
Hi, here's an alternative way of writing it.

VBA Code:
Function ListCalc(pList, Optional pType As String = "Average") As Double
ListCalc = Evaluate(pType & "(" & Replace(pList, " ", ",") & ")")
End Function

Book4
AB
130 40 2090
27575
390 80 80 90 855
450 40 30 20 2232.4
550 40 30 20 2232.4
Sheet1
Cell Formulas
RangeFormula
B1B1=ListCalc(A1,"sum")
B2B2=ListCalc(A2,"Sum")
B3B3=ListCalc(A3,"count")
B4B4=ListCalc(A4)
B5B5=ListCalc(A5,"average")
 
Solution

RoryA

MrExcel MVP, Moderator
Joined
May 2, 2008
Messages
36,696
Office Version
  1. 365
  2. 2019
  3. 2016
  4. 2010
Platform
  1. Windows
  2. MacOS

ADVERTISEMENT

It's not particularly efficient to do all the calculations regardless of which one is actually needed, but other than that it looks ok to me.
 

JenniferMurphy

Well-known Member
Joined
Jul 23, 2011
Messages
1,447
Office Version
  1. 365
Platform
  1. Windows
I guess you are already aware that there are already existing functions in Excel which will do what your UDF does?

. . .

All that said, there I'll reiterate my first point - there are already functions for doing what you describe here in Excel, and I'd be absolutely shocked if it was possible to get even close to the same efficiency from a UDF as from a native function.

- edit - On reading your post a bit closer, I see that you have several numbers in each cell, which makes the approach you're taking more understandable. That will teach me to just skim the source data. I think you may still be better off just using worksheet-functions (sum, average, counta, etc.) on the array you end up with though. I believe all of these will take an array as their argument
Maybe I'm missing something, but I don't think those workspace functions work on a cell with multiple values.
 

JenniferMurphy

Well-known Member
Joined
Jul 23, 2011
Messages
1,447
Office Version
  1. 365
Platform
  1. Windows

ADVERTISEMENT

Hi, here's an alternative way of writing it.

VBA Code:
Function ListCalc(pList, Optional pType As String = "Average") As Double
ListCalc = Evaluate(pType & "(" & Replace(pList, " ", ",") & ")")
End Function
Wow! That works!! Thank you!!!

But what's really amazing is that the pType keywords that I chose just happen to be the correct Evaluate keywords.

Here's my new code. Now it only does the specified calculation and no looping! (y)
VBA Code:
Function ListCalc(pList, Optional pType As String = "Average")

Const ValidTypes As String = "AVERAGE SUM COUNT"

If InStr(1, ValidTypes, pType, 1) = 0 Then
  ListCalc = CVErr(xlErrValue)
  Exit Function
End If

ListCalc = Evaluate(pType & "(" & Replace(pList, " ", ",") & ")")

End Function

And here's some test data:
1608659125724.png


Just one problem. The InStr function conveniently returns a match if any portion of the pType string matches. Unfortunately, Evaluate requires the full type name. So in F10, passing "ave" as the type passes the InStr test, but fails the Evaluate call.

Is there an easy way to check if pType matches any of the valid types exactly?

Thanks
 
Last edited:

JenniferMurphy

Well-known Member
Joined
Jul 23, 2011
Messages
1,447
Office Version
  1. 365
Platform
  1. Windows
It's not particularly efficient to do all the calculations regardless of which one is actually needed, but other than that it looks ok to me.
Agreed, but I was too lazy to add the extra code. But now I have a solution using Evaluate that eliminates that problem.
 

JenniferMurphy

Well-known Member
Joined
Jul 23, 2011
Messages
1,447
Office Version
  1. 365
Platform
  1. Windows
I think I found a solution. I just needed to add a space after the last valid type ("COUNT ") and then add a space after pType in the InStr statement.

Here's that code, which seems to work:
VBA Code:
Function ListCalc(pList, Optional pType As String = "Average")

Const ValidTypes As String = "AVERAGE SUM COUNT "

If InStr(1, ValidTypes, pType & " ", 1) = 0 Then
  ListCalc = CVErr(xlErrValue)
  Exit Function
End If

Select Case UCase(Left(pType, 1))
    Case "A": pType = "AVERAGE"
    Case "S": pType = "SUM"
    Case "C": pType = "COUNT"
End Select

ListCalc = Evaluate(pType & "(" & Replace(pList, " ", ",") & ")")

End Function

Thanks to everyone who contributed.
 

Watch MrExcel Video

Forum statistics

Threads
1,129,478
Messages
5,636,564
Members
416,923
Latest member
jarri

We've detected that you are using an adblocker.

We have a great community of people providing Excel help here, but the hosting costs are enormous. You can help keep this site running by allowing ads on MrExcel.com.
Allow Ads at MrExcel

Which adblocker are you using?

Disable AdBlock

Follow these easy steps to disable AdBlock

1)Click on the icon in the browser’s toolbar.
2)Click on the icon in the browser’s toolbar.
2)Click on the "Pause on this site" option.
Go back

Disable AdBlock Plus

Follow these easy steps to disable AdBlock Plus

1)Click on the icon in the browser’s toolbar.
2)Click on the toggle to disable it for "mrexcel.com".
Go back

Disable uBlock Origin

Follow these easy steps to disable uBlock Origin

1)Click on the icon in the browser’s toolbar.
2)Click on the "Power" button.
3)Click on the "Refresh" button.
Go back

Disable uBlock

Follow these easy steps to disable uBlock

1)Click on the icon in the browser’s toolbar.
2)Click on the "Power" button.
3)Click on the "Refresh" button.
Go back
Top