Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for single item generation #5

Closed
jkonecki opened this issue Sep 25, 2014 · 8 comments
Closed

Support for single item generation #5

jkonecki opened this issue Sep 25, 2014 · 8 comments

Comments

@jkonecki
Copy link
Collaborator

Curently RandomGen supports creation of generators (Func<T>) that can be used to obtain a series of random items.

I found that in most cases I require a single random item at a time.

The solution is to either store the generators (Func<T>) and reuse them between calls or to evaluate the function instantly (Gen.Random.Numbers.Integers()()) which doesn't look nice in code ()().

What I would like to be able to do is write just:

var number = Gen.Random.Numbers.Integers();.

I would like to propose that we create an additional set of methods that would compliment the existing ones and allow for single items to be generated:

Func<int> Gen.Random.Numbers.Integers()
int Gen.Random.Number.Integer()

@aliostad
Copy link
Owner

aliostad commented Oct 1, 2014

Sorry do not quite get what you mean by "I require a single random item at a time".

Also I personally do not have a problem with ()().

But I do have a couple of problems with this:

  • this can lead to confusion and on the fact how to use it
  • people can simply use this overload over the function which will be wrong since we have an instance of Random as a closure in the Func while this will lead to creating the random object all the time.
  • In fact designing this as a Func was to ensure every func gets its own random object so the data are as random as System.Random can provide.

I feel this is pretty much done and don't think tinkering more with it could be useful.

@jkonecki
Copy link
Collaborator Author

jkonecki commented Oct 1, 2014

Regarding bullet points 2 and 3:

The closure of Random and the fact that a new instance of Random was created for every Func was actually an issue - I've fixed it by reusing the single instance of Random for every Func.

The problem was related to the fact that the default constructor of Random seeds the algorithm with the system clock dependent value (clock ticks) and creating multiple instances of Random one after the other would cause all of them to generate the same series of numbers.

On most Windows systems, Random objects created within 15 milliseconds of one another are likely to have identical seed values. (http://msdn.microsoft.com/en-us/library/system.random(v=vs.110).aspx)

I do understand that you're fine with ()().

What I mean by "I require a single random item at a time" is that I need only 1 random item at a time, not the whole series of them. As an example think of a message handler that needs to get a single random country:

public void Handle(Foo message)
{
    message.Country = Gen.Random.Countries()();
}

I personally would rather write:

public void Handle(Foo message)
{
    message.Country = Gen.Random.Country();
}

The above is a matter of personal preferences but also IMHO the principle of the least surprise (http://www.faqs.org/docs/artu/ch11s01.html). I can imagine that ()() construct may be confusing for some on first encounter.

On the other hand I don't think that the following two methods can be confusing due to singular / plural naming and different return types. They read very 'English' and natural to me.

Func<string> Gen.Random.Countries()
string Gen.Random.Country()

@aliostad
Copy link
Owner

aliostad commented Oct 6, 2014

No, it was correct - why have you changed it to static?!

Please note that the creation of Random object is outside the closure:

var random = new Random();
return () => (random.NextDouble() < 0.5);

@jkonecki
Copy link
Collaborator Author

jkonecki commented Oct 6, 2014

Yes, but calling two Func methods one after other would create 2 Randoms with the same seed:

var integers  = Gen.Random.Numbers.Integers();
var doubles = Gen.Random.Numbers.Doubles();

Also, MSDN documentation has the following section regarding multiple initialisations:

Avoiding multiple instantiations

Initializing two random number generators in a tight loop or in rapid succession creates two random
number generators that can produce identical sequences of random numbers. In most cases, this
is not the developer's intent and can lead to performance issues, because instantiating and
initializing a random number generator is a relatively expensive process.

Both to improve performance and to avoid inadvertently creating separate random number
generators that generate identical numeric sequences, we recommend that you create one
Random object to generate many random numbers over time, instead of creating new Random
objects to generate one random number.

@aliostad
Copy link
Owner

aliostad commented Oct 6, 2014

Exactly! That is why we return a factory not the random number itself. Random is not thread-safe and we should not use a single static Random object.

I think we can randomise the seed so we are both happy... how about that? Or actually basing it on RNG...

@aliostad
Copy link
Owner

aliostad commented Oct 6, 2014

One of the reasons I did not RNG to begin with was that it is a disposable resource and I did not want any of that headache.

So I think this should be pretty fine:

new Random(BitConverter.ToInt32(Guid.NewGuid().ToByteArray(), 10))

@jkonecki
Copy link
Collaborator Author

jkonecki commented Oct 6, 2014

I'll make the changes necessary.

I presume you're not interested in single item API?

@aliostad
Copy link
Owner

aliostad commented Oct 6, 2014

I think it is a good idead but honestly it can make it confusing on how to use the API. We should drive them to the pit of success 🎱

@jkonecki jkonecki closed this as completed Oct 6, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants