> public static T Sample<T>(this IEnumerable<T> values) { return Sample(values, 1).First(); } Nice helper method. ---- > if(!values.Any()) { throw new InvalidOperationException("Collection empty."); } I would have used an `ArgumentOutOfRangeException`, but that's a matter of personal preference. Should it throw an exception if `size == 0` or return an empty list? That's a tricky edge case which should be clearly documented. ---- > public static IEnumerable<T> Sample<T>(this IEnumerable<T> values, int size) I would prefer the `Random` to be an argument too, even if it's as `Random rnd = null` with a fallback inside the method. Being able to seed your random selection is important for testing the code and for reproducing results if it's a scientific project. ---- > if (size < 1) { return new List<T>(); } I would definitely prefer to throw an `ArgumentOutOfRangeException` if `size < 0`. ---- > if (size >= values.Count()) { return values; } If `size > values.Count()` then I think that should be `ArgumentOutOfRangeException` or `InvalidOperationException`. Again, whichever way you finally decide, it should be clearly documented. Note that we've now called two methods which iterate partially or fully through `values`. If it was a non-deterministic iterator we have problems. An example of a non-deterministic iterator would be public IEnumerable<int> Demo() { var rnd = new Random(); for (int i = rnd.Next(10); i > 0; i--) yield return rnd.Next(); } If you call this once and then loop through the resulting iterator multiple times then it will not consistently return the same values. You may think that in real usage you won't come across iterators like that, but sometimes the non-determinism is a lot more subtle. ---- > var samples = new List<T>(); var source = new List<T>(values); Since we have to convert `source` to a list here, there's an argument for doing it earlier and ensuring that we only iterate through `values` once, solving the problem of non-deterministic iterators. ---- > source.Remove(value); This is expensive. It's asymptotically more efficient to replace this with source[index] = source[source.Count - 1]; source.RemoveAt(source.Count - 1); ---- Finally, on the names. In your context it might be obvious that `Sample` means "*sample without replacement*", but in some contexts it might be important to distinguish between sampling with and without replacement. Consider whether you might need to distinguish the cases in future usage of the library. ---- Postscript: t3chb0t's suggestion of a `Randomise` method which can then be combined with `Take` is compatible with a rewrite of your `GetSamples` method, and in fact this is what I have kicking around in my library of utility methods (although I call it `Shuffle`). As a modification of your code: public static IEnumerable<T> Randomise<T>(IEnumerable<T> values, Random rnd = null) { if (rnd == null) rnd = _rnd; var source = new List<T>(values); while (source.Count > 0) { var index = rnd.Next(source.Count); yield return source[index]; source[index] = source[source.Count - 1]; source.RemoveAt(source.Count - 1); } }