Avoiding out of range issues in generic functions

Question

I come from the Wild Wild West of PHP and Javascript where you can return anything from a function. While I do hate that lack of accountability, I also face a new challenge in my effort to keep my code "perfect".

I made this generic function to pick a random element from a list

public static T PickRandom<T>(this IList<T> list) {
    Random random = new Random();
    int rnd = random.Next(list.Count);
    return list[rnd];
}

But I want to protect myself from using it on a list with 0 values. Obviously I cannot return anything from this function other than T, such as false or -1. I can of course do this

if(myList.Count > 0)
   foo = Utilites.PickRandom(myList);

However there are so many crazy things in C# that I don't know about, and for this app I am creating I very, very often have to choose a random element from a list that could be constantly decrementing in its Count. Is there a better way?

Answer 1

The options you have are

return default(T)

which will be an ambiguous behavior, as that might be a valid element of the list.

Or you can return something like -1 as you said, but that's quite coupled to your code.

Or you can return null, but that can only be done if T is a nullable type, of course.

In all previous cases, if the caller is not aware of this situation, the application may continue with an invalid value, leading to unknown consequences.

So probably the best option is to throw an exception:

throw new InvalidOperationException();

With this approach, you fail fast and you make sure nothing unexpected occurs beyond the caller's intentions.

One reason more to back up this option. Take for example Linq's extension methods. If you call First(), Single() or Last() on an empty list, you will get an InvalidOperationException with the message "Sequence contains no elements". Giving your class a behavior similar to the framework classes' is always a good thing.

---

I'm adding a side note thanks to Alexei Levenkov's comment in the question. That random generation is not the best approach. Take a look at this question.

---

Second side note. You are declaring your function as an extension method for IList<T> (you do that by using the this before the first parameter) but then you call it just like a static helper method. An extension method is a syntactic sugar that, instead of doing this:

foo = Utilites.PickRandom(myList);

lets you do this:

foo = myList.PickRandom();

More info on extension methods can be found here.