3

Using C#/Asp.Net.

I'm trying to achieve the following:

I have a list of price quotes - sometimes there are multiple products with the same price.

Also, some of the results are affiliated (sponsored), so we need to give a preference to those too.

Here's the method that is called:

 public IEnumerable<PriceQuote> BestQuote(int take = 0) { var q = Quotes.Where(x => x.TotalRepayable == MinPrice) .Shuffle() .OrderByDescending(x => x.ProductDetail.Product.IsSponsored); return take == 0 ? q : q.Take(take); }

The code selects items that have the lowest available price. The idea is then to sort them into a completely random order, then sort again by the sponsored flag descending (sponsored = 1 as opposed to 0), then take however many results are required.

I first shuffle them to get a random order - from the random list I want to take the sponsored items first - then if necessary fill the spaces with non sponsored items. The theory is that both sponsored and non-sponsored will be in a random order each time.

Example in natural order: product1 (not sponsored) product2 (sponsored) product3 (not sponsored) product4 (sponsored) product5 (not sponsored) product6 (sponsored) Shuffle randomly: product3 (not sponsored) product1 (not sponsored) product2 (sponsored) product6 (sponsored) product5 (not sponsored) product4 (sponsored) Order by sponsored first keeping randomness: product2 (sponsored) <-- pick these first product6 (sponsored) product4 (sponsored) product3 (not sponsored) product1 (not sponsored) product5 (not sponsored)

Here's my Shuffle method:

 public static IEnumerable<T> Shuffle<T>(this IEnumerable<T> @this) { if (@this.Count() <= 1) return @this; return @this.ShuffleIterator(new Random()); } static IEnumerable<T> ShuffleIterator<T>(this IEnumerable<T> source, Random rng) { var buffer = source.ToList(); for (int i = 0; i < buffer.Count; i++) { int j = rng.Next(i, buffer.Count); yield return buffer[j]; buffer[j] = buffer[i]; } }

The issue I have is that when I call the BestQuote method a number of times in succession for different quotes, I tend to get the same results returned. For instance, my list contains 6 products and I make 3 calls selecting the first result each time, chances are that the order is the same for all 3 calls. It' is not always the case - there are some variances, but there are more matches than non matches.

Call 1: product2 <-- Call 2: product2 <-- Call 3: product2 <-- this is a common scenario where there seems to be no randomness
Improve this question
9
  • 3
    stackoverflow.com/questions/767999/… Commented Jan 17, 2017 at 12:11
  • 2
    What happens when you use a single static Random instance? Without params new Random() uses the current time as seed, and if you instantiate those close together, you get the same time = same seed = same values. Commented Jan 17, 2017 at 12:11
  • 1
    First, check this almost-duplicate question for correct shuffle implementations. Second, Random returns a float that is scaled to the range you request. If your list contains 4 items, there's a 25% chance that the next double will be scaled to the same integer. You'd get better results if you requested a large integer then took its modulo against the count, eg rng.Next(int.MaxValue)%buffer.Count Commented Jan 17, 2017 at 12:11
  • 1
    Random() uses a time based seed. If you call your method quickly, you will get the same result. You should use a static Random() class. Commented Jan 17, 2017 at 12:17
  • 1
    @PanagiotisKanavos No longer true where - in NET Core. Still applies to full .NET Framework. Commented Jan 17, 2017 at 12:25

2 Answers 2

Reset to default
2

Try this :

 public static IEnumerable<T> Shuffle<T>(this IEnumerable<T> @this) { if (@this.Count() <= 1) return @this; Random rand = new Random(); return @this.Select(x => new { x = x, r = rand.Next() }).OrderBy(x => x.r).Select(x => x.x); }
Improve this answer
Sign up to request clarification or add additional context in comments.

4 Comments

This will only generate a random number once for each object. +1
Why would you want more than one random number per object?
I'm not. Other answers did. Your answer was the better answer, it was a compliment.
You should push the declaration of new Random() out of the method as a static class-level variable. Otherwise if Shuffle is called in quick succession the results won't be random.
-1

I do random sorting like this:

().OrderBy(p => Guid.NewGuid())

So there is a unique and random Guid per item and in every call you can get totally different sorted IEnumerable.

In your case here is how i would do, without any extension methods: 

public IEnumerable<PriceQuote> BestQuote(int take = 0) { var q = Quotes.Where(x => x.TotalRepayable == MinPrice) .OrderBy(x => Guid.NewGuid()) .ThenByDescending(x => x.ProductDetail.Product.IsSponsored); return take == 0 ? q : q.Take(take); }

I am not sure in which order the orderings should be, maybe both the same but if the above code doesn't work you can try like this way:

public IEnumerable<PriceQuote> BestQuote(int take = 0) { var q = Quotes.Where(x => x.TotalRepayable == MinPrice) .OrderBy(x => x.ProductDetail.Product.IsSponsored) .ThenByDescending(x => Guid.NewGuid()); return take == 0 ? q : q.Take(take); }

EDIT

Thanks to @Maarten, here is the final solution with using an extension:

public static IEnumerable<T> Shuffle<T>(this IEnumerable<T> @this) { if (@this.Count() <= 1) return @this; return @this.Select(x => new { x = x, g = Guid.NewGuid() }).OrderBy(x => x.g).Select(x => x.x); }

If you have a few items in your list, it doesn't matter if you use my first or last solution. But as @Maarteen's warning in the comments, there could be a lot more unnecessary Guids than the items' count. And it could be a problem doing multiple comparisons with many items. So i combined @jdweng's answer with mine.

Improve this answer

5 Comments

This will work, but be aware that a new guid will be created for every object for every comparison. So a lot more guids will be created then objects. Better is to first generate a guid for each objects, then use that to sort it.
@Maarten you are right. A Guid property in the Quote class would be better.
Better would be to use an anonymous type with the guid and the object. See jdweng's answer: stackoverflow.com/a/41697115/261050
Thank you @Maarten. This is much better now. I was always lazy to make an extension method for random sorting and now i have one :)
Guid.NewGuid() is not guaranteed to be random. It is only guaranteed to be unique. It should not be used to generate randomness.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.