Are Randomizer's number ranges inclusive or exclusive?

bchavez / Bogus

:card_index: A simple fake data generator for C#, F#, and VB.NET. Based on and ported from the famed faker.js.

Other

8.51k stars 483 forks source link

Are Randomizer's number ranges inclusive or exclusive? #528

Closed Mitchman215 closed 5 months ago

Mitchman215 commented 5 months ago

Bogus NuGet Package

34.0.2

.NET Version

.NET 8

Visual Studio Version

No response

What operating system are you using?

Windows

What locale are you using with Bogus?

en_US

What's the problem?

It doesn't seem to be clearly documented whether the number ranges in Faker.Random are inclusive or exclusive. For example, Random.Double's documentation reads "Get a random double, between 0.0 and 1.0."

Provide LINQPad Example

What other possible solutions or alternatives have you considered?

Looked at README docs and individual function docs

bchavez commented 5 months ago

You can also check the source code:

https://github.com/bchavez/Bogus/blob/1774a851db082b5071291620f35bf8d53fd4dc15/Source/Bogus/Randomizer.cs#L173-L191

Looks like in this specific case, Randomizer.Double() defers to System.Random.NextDouble():

https://learn.microsoft.com/en-us/dotnet/api/system.random.nextdouble?view=net-8.0

Mitchman215 commented 5 months ago

Thanks for the quick answer! Would you be opposed to me opening up a PR adding this documentation, just so it's a bit easier to determine?

bchavez commented 5 months ago

@Mitchman215 , of course, go for it.

Just a couple of guidelines:

Reference this GH issue number #528 in your PR.
Also, feel free to make the same kind of doc improvements (inclusive/exclusive) in Randomizer.cs if they are missing in other XML param docs for other methods.
- But please keep your PR/change-set strictly focused on this kind of change (ie: don't include any code reformatting); keep the PR/changes strictly on topic.

I might not be able to immediately merge/approve it tonight; I'm just about to turn in for the night. But will review/approve/merge probably sometime tomorrow.