I'm going to suggest that you be Linq-minded
and create a good, general purpose IEnumerable<T>
extension method that does the heavy lifting you require and then your GetRandomColor
functions are simpler and you can use the extension method for other similar tasks.
So, first, define this extension method:
public static IEnumerable<T> SelectRandom<T>(this IEnumerable<T> @this, int take)
{
if (@this == null)
{
return null;
}
var count = @this.Count();
if (count == 0)
{
return Enumerable.Empty<T>();
}
var rnd = new Random();
return from _ in Enumerable.Range(0, take)
let index = rnd.Next(0, count)
select @this.ElementAt(index);
}
This function allows you to select zero or more randomly chosen elements from any IEnumerable<T>
.
Now your GetRandomColor
functions are as follows:
public static MyColor GetRandomColour()
{
return AvailableColors.SelectRandom(1).First();
}
public static MyColor GetRandomColour(IEnumerable<MyColor> except)
{
return AvailableColors.Except(except).SelectRandom(1).First();
}
The second function accepts an IEnumerable<MyColor>
to exclude from your available colors so to call this function you need to select the MyColor
property from your collection of items. Since you did not specify the type of this collection I felt it was better to use IEnumerable<MyColor>
rather than to make up a type or to define an unnecessary interface.
So, the calling code looks like this now:
var myRandomColor = GetRandomColour(collectionOfItems.Select(o => o.MyColor));
Alternatively, you could just directly rely on Linq and the newly created extension method and do this:
var myRandomColor =
AvailableColors
.Except(collectionOfItems.Select(o => o.MyColor))
.SelectRandom(1)
.First();
This alternative is more readable and understandable and will aid maintainability of your code. Enjoy.