excel vba discrete random variable udf
Option Explicit

Option explicit. Always.

Strong typing

VBA facilitates strong typing. This should be used because it stops VBA guessing at types which improves performance and reduces the chance of coding errors. Especially as some types/objects have default values that are not well documented!

This is a function, so what does it return? At the moment, it returns a Variant which might be OK, but your code does not reflect this requirement.

Public Function discrete(r1 As Range, r2 As Range) As String


If you want it to be Variant, then be explicit and state that!

Also, another gotcha is the way variables are declared. The following lines are not doing what you think they are doing:

Dim arr1, arr2 As Variant
Dim min, max As Double


In these lines, both the first variables are not declared and thus they default to Variant. Again, if you want a variable to be a Variant, then be explicit (like you tried to be in the first line above). What you really wanted was:

Dim arr1 As Variant, arr2 As Variant
Dim min As Double, max As Double


By the way, kudos for declaring variables near where you are using them.

Proper qualification

Always put proper qualification on your objects. Excel uses defaults for things that are not properly qualified - the active sheets and cells may not be what you want! Especially in a UDF and you could have multiple workbooks open!

Range(r2(1), r2(1).End(xlDown))


should be

wb.ws.Range(r2(1), r2(1).End(xlDown)) ' wb and ws is something you define here - more notes below.


Default value for discrete

You only set the value for discrete in a conditional (If) statement in a loop. What if it never fires? What would you like the user to see?

A simple option is to set a default value at the beginning - the most obvious is an error message. For this to work, the UDF should return a variant:

Public Function discrete(r1 As Range, r2 As Range) As Variamt
    discrete = CVErr(xlErrValue) '&lt;-- pick an error value to suit yourself


Now that we have set that up - what are some other ways this can fail?


user picks ranges that re not one column wide?
user picks two ranges that are a different size?


Now you can test for this early and then exit the function - the default value will be an error and the user will have an indicator of this.

Altering the parameters

This is bad:

Function discrete(r1 As Range, r2 As Range)
    […]
    Set r1 = Range(r1(1), r1(1).End(xlDown))


A big no-no. You should always consider the parameters on a UDF as immutable. If you want to reset the ranges, then use a temporary variable within the Function scope (e.g. Dim tempRange1 as Range).

However, you are double-guessing the user. What if they did not want to go to the end of the page? What if they had other stuff under the table?

By doing this manipulation, you are increasing the chances that the value arrays will be mismatched. arr1(found,1) may no longer have an equivalent arr2(found,1)!

Working with arrays

Yes, always practice working with arrays where you can. Even replace the Application.Sum call. You can never tell when your user is going to set up a 1000-value discrete array!

Some suggestions


Use only a single range in the UDF call (Public Function
discrete(twoColTable As Range) As Variant. This reduces the
opportunity for user error. You array will be two columns
arr(found,1) will correspond with arr(found,2)
Check your user input immediately for size and values to make sure
you are getting what you think you should. If column count does not
equal 2, or your second column is not ascending numbers then exit
with an error.
Traverse the array from beginning until you find a value that is
greater then your n. The value you want will be the one before you hit that value (i.e. go back one array index). Alternatively, traverse the array from the end until you find a value that is not greater than your n. This way, you do not need max or min.