1

I have this class in my infrastructure that suppose to return random image. It always returns same image. I have exactly same code used in different place on my website and it works. Any ideas?

This question is where I got the info for getting random value. I don't understand why it works on one place and not another though...

Background.cs

public static class Background
{
    public static string Get()
    {
        photoBlogModelDataContext _db = new photoBlogModelDataContext();
        var image = _db.Images.OrderBy(x => Guid.NewGuid()).FirstOrDefault();
        return image.Small; // Always same value?
    }
}

Same code on another page that works where I loop through my gallery and choose random image from it

<img src="@Url.Content("~/content/uploads/" + item.Images.OrderBy(x => Guid.NewGuid()).FirstOrDefault().Small)" alt="" />
Community
  • 1
  • 1
Stan
  • 25,744
  • 53
  • 164
  • 242
  • Are you sure you've got more than one Image in `_db.Images` ?? – Aren May 24 '12 at 17:15
  • http://blogs.msdn.com/b/oldnewthing/archive/2012/05/23/10309199.aspx – Lee May 24 '12 at 17:18
  • Why would anyone think the Guid is random? It is not random, only unique. Those are completely different things. – Tomas Voracek May 24 '12 at 17:22
  • @Aren 100% positive. As I said before it works on code below. I have total 3 records of images in my database. – Stan May 24 '12 at 17:24
  • @TomasVoracek Well, algorithm 4 for GUID generation does generate a random number for one component. it's still doesn't make GUIDs a good choice for creating a random number, but it can create a logical connection in some people's minds. GUIDs do however make great SEEDS for a random number generator as they won't be the same even if called in very quick succession (as opposed to system time). – Servy May 24 '12 at 17:24

3 Answers3

2

Guids are not random numbers. They are often sequential as they have a timestamp component, which means that you will always be getting the first or last image from that code. You should use the Random class (or one of the cryptographic random number generators if it's really important) to get a random number between 0 and the number of images you have and then take the n-th image (where n is the random number). If you call this function more than once in a short time span then you should make sure that you use the same instance of Random between all of the calls to the method. (That means making a private static Random instance that you re-use.)

Servy
  • 202,030
  • 26
  • 332
  • 449
0

Well, I would say that one time you have linq2entities, and one time linq2object

Not sure that the OrderBy(Guid.NewGuid()) works the same.

If you try to enumerate

public static class Background
{
    public static string Get()
    {
        photoBlogModelDataContext _db = new photoBlogModelDataContext();
        var image = _db.Images.ToList().OrderBy(x => Guid.NewGuid()).FirstOrDefault();
        return image.Small; // Always same value?
    }
}

it should change.

Raphaël Althaus
  • 59,727
  • 6
  • 96
  • 122
  • This will have the same problem. – Servy May 24 '12 at 17:19
  • Hmmm I would give a try anyway. Not saying it's the perfect randomness, but @Qmal seems to have "working" code in another part of its site. – Raphaël Althaus May 24 '12 at 17:21
  • @Qmal Do you want to pull every single image into memory for your program? Once you have more than 3 images that could be a bit of a problem. Not to mention the fact that GUIDs still aren't random numbers... – Servy May 24 '12 at 17:26
  • @Servy never thought about it. What should be a good way to handle this? BTW randomness of GUID is the least of my concerns, it seems to work fine, I don't have to get perfectly random image, I just want them to change from time to time. I will also put caching on this function so it will get random image every minute or so... – Stan May 24 '12 at 17:30
  • @Qmal Well, I provided an answer to this question that specifically tells you how I think you should handle this. When I say a GUID isn't random I don't mean it's a random number that is simply more likely to generate one number over another. As my answer states, GUIDs are based off of (in large part) a timestamp, so each guid is larger than the one before it, so if you order by sequentially generated guids the first one will always be the smallest, meaning that code always returns the first image. Clearly that's not what you want. – Servy May 24 '12 at 17:34
  • @Servy test it in a Console app ! Three order by on a five elements list with orderby(x => Guid.NewGuid()) gives me three different orders. (in linq2objects). But I believe this is not the same in sql (linq2entities) – Raphaël Althaus May 24 '12 at 17:35
  • @RaphaëlAlthaus it depends on the algorithm that that computer is using to generate GUIDs. Did you read the link that I provided in my answer? It discusses the issue in detail. – Servy May 24 '12 at 17:42
  • @Servy. I see : **The second is that it is possible for GUIDs generated on a particular machine with this algorithm to be monotone increasing** But any idea of the probability of that possibility (not ironic, real question) ? – Raphaël Althaus May 24 '12 at 17:51
  • @RaphaëlAlthaus When using algorithm #1, 100%. (or close to it anyway) – Servy May 24 '12 at 17:53
  • @Servy : yes, but we're talking of method Guid.NewGuid() in c#. Then ? – Raphaël Althaus May 24 '12 at 17:59
  • @RaphaëlAlthaus Yes. You don't get to choose the method of generating the GUID, your computer does. That same code will behave differently on different machines. It is partly because of this unreliability that you shouldn't use GUIDs as random numbers. Is there some compelling reason that you're opposed to using the `Random` class, which is designed for doing this? Even if you can get it to work once or twice with a GUID, why not just do it right and be done with it? – Servy May 24 '12 at 18:00
  • @Servy : nono, nothing against random ;) I have just read http://stackoverflow.com/questions/654906/linq-to-entities-random-order too much ! – Raphaël Althaus May 24 '12 at 18:19
  • @RaphaëlAlthaus You want to get one random item, not all items in a shuffled order. It's a much easier problem. – Servy May 24 '12 at 18:33
0

Is there a reason why you cannot use the System.Random class like below?

var random = new Random();
var index = random.Next(0, count); // use the list count here
var randomImage = _db.Images[index]; // or equivalent
Channs
  • 2,091
  • 1
  • 15
  • 20