How to generate a random number in the constructor of a class in C#

Question

I know that it is not a good practice to call a method in the constructor of a class in C# but I stuck on something strange. My problem is that when I create an object of my class I need to assign a field in my object with a random number.

for instance

class RandomNumberHandler
    {
        private int randomNumber;
        public RandomNumberHandler()
        {
            this.randomNumber = GenerateRandomNumber();
        }

        private int GenerateRandomNumber()
        {
            return (new Random()).Next(3000) + 1000;
        }
    }

In my case I need a four digit number. I thought of generating the random number in the class where I am creating the object and passing it as a parameter to the constructor but generating a random number in the other class does not seem a very good idea either because I am trying to achieve strong cohesion for my classes. I am doing this for a "High quality code" course in my university and I am looking for the best approach. Any ideas how to do this are welcome :)

It's not a good practice to call a **virtual** (including abstract) method **of the object being constructed** in a constructor. Calling methods is fine, and indeed normal. — Jon Hanna, Mar 24 '11 at 16:27

Jon · Accepted Answer · 2011-03-24T16:23:19.203

7

First off: there is nothing wrong with calling non-virtual methods in the constructor. Where did you read that there was? (Note: calling virtual methods can be a problem; it is not an automatic no-no, but you need to watch what you are doing very carefully).

As an aside, it seems wasteful to generate a new Random instance every time GenerateRandomNumber is called. You can extract the Random instance to a field to fix that:

class RandomNumberHandler
{
    private readonly Random random = new Random();
    private int randomNumber;

    public RandomNumberHandler()
    {
        this.randomNumber = GenerateRandomNumber();
    }

    private int GenerateRandomNumber()
    {
        return this.random.Next(3000) + 1000;
    }
}

But this raises another question: if GenerateRandomNumber is only called once in each instance's lifetime (in the constructor), then it doesn't make sense to create a new Random for each object. So the next logical step is to make random be static. This means that GenerateRandomNumber can also become static (and indeed, it has to):

class RandomNumberHandler
{
    private static readonly Random Random = new Random();
    private int randomNumber;

    public RandomNumberHandler()
    {
        this.randomNumber = GenerateRandomNumber();
    }

    private static int GenerateRandomNumber()
    {
        return Random.Next(3000) + 1000;
    }
}

edited Mar 24 '11 at 16:23

answered Mar 24 '11 at 16:13

Jon

428,835
81
738
806

Who knows... these teachers always have to impose their opinions of how things should be done. I just always go with the flow for the grade. – Pete Mar 24 '11 at 16:16
There are some considerations to calling *virtual* methods in a constructor though. – Kirk Woll Mar 24 '11 at 16:17
Possibly he is confused about virtual methods. – Eric Mar 24 '11 at 16:17
@Kirk, @Eric: Sure. But the point is to get the OP to express that in his own words so that he can improve his understanding. – Jon Mar 24 '11 at 16:19
@Jon, perhaps. But this statement (in bold no less), "there is nothing wrong with calling methods in the constructor" strikes me as wrong or at best incomplete. – Kirk Woll Mar 24 '11 at 16:20
@Kirk: You 're right on that. I changed it a bit and added a note about calling virtuals. – Jon Mar 24 '11 at 16:23

score 4 · Answer 2 · answered Mar 24 '11 at 16:16

This code will work okay - except that you'll likely get the "same" random number if you call it in quick succession.

You could, of course, easily work around that by using a static random number (with a lock for thread safety if you're using multiple threads), such as:

class RandomNumberHandler
{
    private static Random random = new Random();
    private static object syncObj = new object();

    private int randomNumber;
    public RandomNumberHandler()
    {
        this.randomNumber = GenerateRandomNumber();
    }

    private static int GenerateRandomNumber()
    {
        lock(syncObj)
            return random.Next(3000) + 1000;
    }
}

+1 as while it's similar to two other answers, its the only one that's thread-safe. — Jon Hanna, Mar 24 '11 at 16:24
@Jon: When I posted it, it was the only version that was using statics ;) — Reed Copsey, Mar 24 '11 at 16:25

score 1 · Answer 3 · edited May 23 '17 at 12:17

1

Random number generator only generating one random number

Short answer: You should use same Random instance across all Next() calls.

edited May 23 '17 at 12:17

Community

1
1

answered Mar 24 '11 at 16:14

Euphoric

12,645
1
30
44

score 0 · Answer 4 · answered Mar 24 '11 at 16:14

I don't see any cohesion there by just wrapping the Random class inside of another class named RandomHandler. Personally, I think thats awkward. If you need a brand new completely random number then just call Random().Next(3000) or whatever inside of the constructor like you said.

Jordão · Answer 5 · 2011-03-24T16:30:27.937

If you lift the Random instance as a static field and make the GenerateRandomNumber static, you can call it in the declaration of the randomNumber field:

class RandomNumberHandler {
  private static Random random = new Random();
  private int randomNumber = GenerateRandomNumber();

  private static int GenerateRandomNumber() {
    return random.Next(3000) + 1000;
  }
}

Or more simply (and less readable):

class RandomNumberHandler {
  private static Random random = new Random();
  private int randomNumber = random.Next(3000) + 1000;
}

Although it doesn't look like you're calling a method in a constructor, if you look at the generated CIL, you'll find out that you are.

Also, if you care about thread-safety, take a look at this article.

How to generate a random number in the constructor of a class in C#

5 Answers5

Linked