CODEWITHSUNDEEP
asp.net-mvc
KingNestor
KingNestor

Reputation: 68020

Why is my random number html extension method returning the same values?

We are giving a demo in a couple days and I have to go in and mock-up a lot of our views. This includes making a lot of fake data, etc. I figured I would drop in a loop and an extension method that returns random numbers so I don't have to make up this hard-coded data myself.

Here is my view code:

<% for(int i = 1; i < 7; i++) { %>
  <tr>
    <td class="auditsTableAgencyElement">Agency <%=i %></td>
    <td class="auditsTableResults"><%= Html.GetRandomNumber(0, 30) %></td>
    <td class="auditsTableResults"><%= Html.GetRandomNumber(0, 100) %>%</td>
    <td class="auditsTableResults"><%= Html.GetRandomNumber(0, 20) %></td>

    <% foreach (var record in Model.Categories) { %>
      <td class="auditsTableResults"><%= Html.GetRandomNumber(0, 30) %></td>
      <td class="auditsTableResults"><%= Html.GetRandomNumber(0, 100) %>%</td>
      <td class="auditsTableResults"><%= Html.GetRandomNumber(0, 20) %></td>
    <% } %>
  </tr>
<% } %>

Here is what my view looks like after doing this: alt text

Same numbers down the line. Are my requests for random numbers getting cached and returned to me? If so, how do I turn off this functionality for this method only?

public static string GetRandomNumber(this HtmlHelper html, int low, int high)
{
  Random myRand = new Random();
  return myRand.Next(low, high).ToString();
}

Upvotes: 1

Views: 525

Answers (2)

Dan Atkinson
Dan Atkinson

Reputation: 11728

I refer you to here:

getRandomNumber

:-P

</sarcasm>

Upvotes: 1

Jon Skeet
Jon Skeet

Reputation: 1502985

As ever with this sort of issue, the problem is that you're creating a new Random instance on every iteration.

Create a single instance and reuse it repeatedly. You can use a static variable, but that won't be thread-safe. In this particular case, creating a new instance per page would probably be okay. However, you'll still get the same data if two people access the page at the same time.

Ideally, you could create a static random used in a thread-safe way, to create new instances of Random which can then be used without locking within a single thread. For example:

public static class RandomFactory
{
    private static Random rng = new Random();
    private static readonly object padlock = new object();

    public static Random CreateRandom()
    {
        lock (padlock)
        {
            return new Random(rng.Next());
        }
    }
}

Then in your page you could have:

// Instance variable
protected readonly Random rng = RandomFactory.CreateRandom();

and change your method to:

public static string GetRandomNumber(this HtmlHelper html, Random rng,
                                     int low, int high)
{
    return rng.Next(low, high).ToString();
}

(I'm not quite sure why you've got HtmlHelper at all there, to be honest - you're not using it...)

and finally your mark-up to things like this:

<%= Html.GetRandomNumber(rng, 0, 30) %>

Upvotes: 10

Related Questions