Change the random generation method

[f-alizada]

Context

Changing the usage of the Random to RandomNumberGenerator, to align with the codebase

[rainersigwald]

This is fine, but this whole thing seems pretty heavyweight when we could do something like https://sharplab.io/#v2:EYLgHgbALAPgAgJgIwFgBQcAMACOSB0AIgJYCGA5gHYD2AzgC7EDGtA3OunAMy4LYDC2AN7psY3Dzw4AsgApiletgA2AU0rl6ACwCUw0eMMAVAE6kmq/AEFatVSfqy1G7dgA82Lgh3s0hw3AA7NgA4gCuxAAm+AByqgDu4VGyOvhG1ADK9CYK5LIARDH5qRlhwFKymAA0Kuqaur6GAL7oTUA

[rainersigwald]

Update doc comment above

[danmoseley]

Is there a performance advantage in holding one of these in a static (or thread static)? I don't know, but some implementations of such things generate a block and then return it in pieces as requested. Or pull expensive entropy to start up. @vcsjones ?

[vcsjones]

RandomNumberGenerator was never documented as thread-safe, even though in all practical cases, it is thread-safe when used with RandomNumberGenerator.Create(). Practically speaking, it would work fine to pull this up to a static, even in .NET Framework.

We've made significant investments in the RandomNumberGenerator for .NET (Core). If you want to avoid the allocation for the modern .NET, you can do something like

byte[] randomBytes;

#if NET
  randomBytes = RandomNumberGenerator.GetBytes(randomBytesNeeded);
#else
  randomBytes = new byte[randomBytesNeeded];
  using var rng = RandomNumberGenerator.Create();
  rng.GetBytes(randomBytes);
#endif

[JanKrivanek]

Thank you for suggestions.

We're currently using this utility to create name of a build binary log file - so simpler code is preference

[vcsjones]

but this whole thing seems pretty heavyweight when we could do something like

We've discouraged using Guid.NewGuid() as a source of cryptographic randomness. Though .NET today largely does use cryptographic random values in Guid, that is not really the purpose of GUID. If you need a crypto RNG, use a crypto RNG.

[JanKrivanek]

This is fine, but this whole thing seems pretty heavyweight when we could do something like https://sharplab.io/#v2:EYLgHgbALAPgAgJgIwFgBQcAMACOSB0AIgJYCGA5gHYD2AzgC7EDGtA3OunAMy4LYDC2AN7psY3Dzw4AsgApiletgA2AU0rl6ACwCUw0eMMAVAE6kmq/AEFatVSfqy1G7dgA82Lgh3s0hw3AA7NgA4gCuxAAm+AByqgDu4VGyOvhG1ADK9CYK5LIARDH5qRlhwFKymAA0Kuqaur6GAL7oTUA

AFAIK the individual parts of guids have different distributions (there is a part derived from machine uuid as well I believe - so effectively constant on a single machine) - so the substrings of particular length might not serve the purpose well.

[rainersigwald]

This is used in one place to disambiguate a filename. I would love to delete the code entirely and use a non-cryptographic semi-random thing in its place.

[f-alizada]

Closing the PR: Since the System.Random used for the filename. Thank you for all the comments and reviews

[danmoseley]

for what it's worth, if you are x-targeting to .NET Core, you can switch out Random random = new(); for Random.Shared for a little more convenience/efficiency.