Crashing UDF - debug tips needed

[werafa]

Hi all,
I'm working on a user defined function that selects and sums three months worth of data from an annual data entry spreadsheet

I have ID'd the data range of interest, but the function crashes when I attempt to interact with this range. one offending example is

Set sumRange = mySheet.Range(Cells(lRow, lCol1), Cells(lRow, lCol2))

processing this line causes the vbe to cease code execution entirely
my row and column variables do have valid data and I'm getting confabulated over why this is happening.

can anyone spot an error in my code, give any suggestions on how I might be able to trap the error or advise if there are any special rules with UDF's that I am breaking?

the complete module is:

Public Function SumResult(sOrg As String, sName As String) As Variant
' ----------------------------------------------------------------
' Purpose: sum quarter totals for results columns
' Parameter sOrg (String): ID Org data (start row)
' Parameter sName (String): ID row data to sum
' Return Type: Variant
' ----------------------------------------------------------------

Dim myFinMonth As cls_FinMonth
Dim lMonthFrom As Long
Dim lMonthTo As Long
Dim myResult As Variant
Dim lRow As Long
Dim lastRow As Long
Dim mySheet As Worksheet
Dim sumRange As Range

Dim lCol1 As Long
Dim lCol2 As Long

Set mySheet = ThisWorkbook.Worksheets("2018-19")
Set myFinMonth = New cls_FinMonth 'initialise class

myFinMonth.FinMonthName = ThisWorkbook.Worksheets("Admin").Range("nrMonthName")
lastRow = mySheet.Cells.Find("*", SearchOrder:=xlByRows, SearchDirection:=xlPrevious).Row

'ID month range
lMonthTo = myFinMonth.FinMonthNum
lMonthFrom = myFinMonth.FinQuarter * 3 - 2

'ID data Row
lRow = GetRow(sOrg, sName)

'sum range intersect
lCol1 = lMonthFrom + 3
lCol2 = lMonthTo + 3
Set sumRange = mySheet.Range(Cells(lRow, lCol1), Cells(lRow, lCol2))
'myResult = Application.WorksheetFunction.Sum(sumRange)

If myResult = 0 Then
SumResult = "Code Error"
Else
SumResult = myResult
End If

End Function

[Aflatoon]

This is a common mistake. You need to qualify the Range and Cells properties with the same sheet:


Set sumRange = mySheet.Range(mySheet.Cells(lRow, lCol1), mySheet.Cells(lRow, lCol2))

On a separate note, this is bad design for a UDF, because you have to make it volatile to work properly due to the indirect references to other cells. All necessary ranges should be passed as direct arguments to the UDF ideally.

[Paul_Hossler]

In addition to Aflatoon's comments, I'd try to avoid using a class inside a UDF, most for performance

What is GetRow() ?



An example workbook with a description of the desired results would help

[werafa]

thanks all
this makes sense.

Getrow is a function that takes the two user input fields to determine the correct row to sum, and returns this row number

I take your point re the class inside a udf - this will cause unnecessary replication, and it is probably better to run this once and hold the value as a module level variable. I can maintain this via a worksheet change event. thanks for the tip.

Werafa

[Paul_Hossler]

Ok, but it still seems unnecessarily complex for a UDF that might be used many times on a worksheet

Dumb question -- ARE you planning to use this on a worksheet (which is the definiton of a UDF) or as a function called by other macros?

[werafa]

Hi Paul,
yes I am.

I could easily avoid using the class module, but I recently decided to learn to use them, and this is my second implementation.
there isn't much code in the class - it is an object that synchronises month name, month number and financial month number.

The udf is to be used in a report - so there is little to no user interaction. I will add one more variable so I can point it to a three-dimensional table structure and return one of three answers.

so yes, it is probably not the most efficient code but I will be satisfied if it does the required task. I am also happy that I am beginning to understand how classes work and how powerful they can be.

thanks again for your interest
Werafa

[Jan Karel Pieterse]

Whatever you do, take Aflatoon's advice on passing all ranges as arguments very seriously. In short: Any range of cells the UDF needs should be passed to the UDF as an argument. Using a worksheet change event to handle how a UDF works is a very unusual way of doing things.