Excel VBA - Use of Application.Caller within UDF

It is not good practice to have relative references in a UDF...

So if the UDF needs data from columns 1 and 2 to fill column 3 you would need:

=UDF(FirstCell,SecondCell)  or =UDF(FirstCell, LookupRange)

so you have all the variables entered into the UDF at the start.
calling relative caller - 1.row makes for unexpected and troublesome UDF.

Do you wish to post the full UDF and we can suggest improvements?

I agree totally with The_Barman. If for some reason you have to use this approach, you must specify the sheet for Cells:

x = Application.caller.worksheet.Cells(Application.Caller.row, 4).Value

Thank you both for your quick and very helpful responses.

I have modified the call to the UDF by using Row() to derive the row number which is then passed to the UDF.  I then removed the Application.Caller.row from the UDF and x is then derived as
    x = Cells(Row, 4).Value

When I change the value of a cell in another workbook that is open the #VALUE! appears and x has a value of 'empty', presmably because the UDF is referencing the other workbook.  

Would it be acceptable to use the following (which does appear to work)?  Incidentally, the name of the workbook with the UDF's is not fixed.

x = Application.Caller.Worksheet.Cells(Row,4).Value

Many thanks
Alison

Thank you both again.

Initially I did include arguments for the other cells but I have other UDF's with more arguments, the greatest being 7.  From the user's perspective it looked more compact to have a UDF with no arguments rather than 7.  

I am willing to apply this approach (with an argument passed for each cell) if that is the best and right way.  

Many thanks
Alison

I would probably have a serious rethink about a UDF that needs that many arguments.

The use of 7 arguments can be excessive, maybe the use of a Range or ParamArray would tidy the function up. Array handling and off sheet processing can really speed up the function.

Please feel free to post a version of the function here and I am sure we can provide guidance to improve things for you.

Many thanks The_Barman for the offer.

Here is a copy of the function with 7 arguments.

Function Xval(StartDate As Date, EndDate As Date, Liability As Long, DDate As Date, Instal1 As Long, Instal2 As Long, InstalYN As Integer, _
             Optional default_value As Variant) As Variant
Application.Volatile

Dim ToAlloc As Long
Dim Plength, WholeMths, spare_days, n As Integer

    Plength = EndDate - StartDate + 1
   
    If InstalYN = 1 And IsDate(DDate) Then
        WholeMths = WorksheetFunction.RoundDown(Plength / 30, 0)
       
        spare_days = EndDate - DateAdd("m", WholeMths, StartDate) + 1
        n = WholeMths + Round(spare_days / 30, 2)
        ToAlloc = Liability - Instal1 - Instal2
   
        If Plength = 365 Or Plength = 366 Then
            Xval = 3 * Liability / n
        Else
            If (3 * Liability / n) < ToAlloc Then
                Xval = 3 * Liability / n
            Else
                Xval = ToAlloc
            End If
        End If
    Else
        Xval = 0
    End If
   
End Function

Thanks again, Alison

Erm, sorry to be a pain but would you be able to provide a sample worksheet too, just with 3-4 lines of random data.

I am not sure if you want to dim Plength as a variant but it is a common misconception (one I had until I started on this site) that you can dim with commas and just define at the end.

So:
Dim Plength, WholeMths, spare_days, n As Integer
is in effect:
Dim Plength as variant, WholeMths as variant, spare_days as variant, n As Integer
so should be:
Dim Plength As Integer, WholeMths As Integer, spare_days As Integer, n As Integer

As all dependencies are passed to the function you can drop the Application.Volatile too.
The function as is will calculate on change with the volatile, or only on change of dependency without it.

the Plength = End - Start only comes into effect inside the If = true (so moving it inside will make a slight diference)

If InstalYN = 1 And IsDate(DDate) Then
        'the following calc only comes into effect inside If, so move it inside
        Plength = EndDate - StartDate + 1

Or you can drop the Plength all together with (eliminating a variable)
WholeMths = WorksheetFunction.RoundDown((EndDate - StartDate + 1) / 30, 0)

I will see if there are any other changes with the sample data.
Steve.

Thanks for the comments and suggestions Steve.  

Attached is a new workbook which just contains the relevant cells together with 4 rows of data.  

The UDF is used in cells W9 - W12.  Similar UDF's will be used in other columns but I have not included them in this workbook.

Thanks, Alison

Hi there Alison, missing attachment.

I am sorry Steve.  Not sure why that failed.  I will try again
Function-example.xlsm

A question:
you dim n as integer before using the Round on Spare_days:
n = WholeMths + Round(spare_days / 30, 2)
but as Spare_days will always be less than 30 and n will force no decimals (integer) the round to 2 decimals will have the effect of a Round to 0 decimals. Is this intentional?

Elimination of Worksheet function:
WholeMths can be calculated with the DateDiff function:
This will be more accurate...
WholeMths = DateDiff("m", StartDate, EndDate + 1)

Thanks for this Steve!
n should be declared as long.

Also, thank you for the alternative derivation of WholeMths.  That is better.

Alison

Hi there Alison, you are most welcome...

n would still round to no decimal places if it is Long.

To have n to 2 decimal places it should be Double data type.

ATB
Steve.

thanks for all your help Steve.  

Is there anything I can do instead of using 7 arguments with the UDF?

Thanks, Alison

The use of 7 arguments seems to be nescessary.
(though the optional default is not used in the code)

But it is far far better to pass the arguments into the function this way rather than cell offset.

There are little changes that you could still make, but the function is good as it is.

ATB
Steve.

many thanks again Steve for all your help.  It really is most appreciated.

Thanks, Alison