Skip to main content
edited body
Source Link
J_H
  • 43.3k
  • 3
  • 38
  • 158

magic numbers

The values \$1,000,000,000\$ and \$9\$ are not well motivated. At a minimum they deserve comments or helpful identifier names.

I understand we're talking about a value less than four billion (once I've inserted commas), but it's unclear how that value is related to \$49,007,199,254,740,991\$. Doesn't Math.random() offer 53 bits of entropy? Though this bit of documentation sounds like it could be Mersenne Twister, making me wonder why we would ever wish to call it:

Note: Math.random() does not provide cryptographically secure random numbers. Do not use them for anything related to security. Use the Web Crypto API instead

When an API says "random", that makes me think "indistinguishable", so I naturally gravitate toward crypto secure implementations.

one-liner

There's no point to compressing this pair of assignments down to a single line.

    var rand = [], digits = range. ...

Use another punch card! Put var digits on a line of its own.

cryptic type conversion

The | 0 hack is pointless. Be explicit; call Math.floor() already.

Similar critique for the Math.random() * 1000000000 | 0 expression. If there's some concern you have in mind, say it, don't be oblique about it.

BigInt is about decimal digits

The whole business of working in nine- or ten-digit chunks seems a little suspect. And there's no need for a BigInt() call within a tight loop.

  1. Determine how many digits are necessary; call it \$N\$.
  2. Loop to roll \$N\$ random digits, uniformly distributed from \$0 ~ .. \thinspace 9\$.
  3. [optional step]
  4. Glue those random digits together.
  5. Present the digits to BigInt().
  6. Use rejection sampling to perhaps re-roll.

Using an "obvious" approach like this will make it much easier for future maintenance engineers to immediately see that the library routine satisfies its contract. An optional step might re-roll based on just the high order digit, to avoid some (expensive) BigInt() calls.

standard library

Consider calling getRandomValues to produce a bunch of Uint8Array values. Typically you'll win with a single call. Mask them down to four bits, then do rejection sampling ofto discard values greater than \$9\$.

magic numbers

The values \$1,000,000,000\$ and \$9\$ are not well motivated. At a minimum they deserve comments or helpful identifier names.

I understand we're talking about a value less than four billion (once I've inserted commas), but it's unclear how that value is related to \$49,007,199,254,740,991\$. Doesn't Math.random() offer 53 bits of entropy? Though this bit of documentation sounds like it could be Mersenne Twister, making me wonder why we would ever wish to call it:

Note: Math.random() does not provide cryptographically secure random numbers. Do not use them for anything related to security. Use the Web Crypto API instead

When an API says "random", that makes me think "indistinguishable", so I naturally gravitate toward crypto secure implementations.

one-liner

There's no point to compressing this pair of assignments down to a single line.

    var rand = [], digits = range. ...

Use another punch card! Put var digits on a line of its own.

cryptic type conversion

The | 0 hack is pointless. Be explicit; call Math.floor() already.

Similar critique for the Math.random() * 1000000000 | 0 expression. If there's some concern you have in mind, say it, don't be oblique about it.

BigInt is about decimal digits

The whole business of working in nine- or ten-digit chunks seems a little suspect. And there's no need for a BigInt() call within a tight loop.

  1. Determine how many digits are necessary; call it \$N\$.
  2. Loop to roll \$N\$ random digits, uniformly distributed from \$0 ~ .. \thinspace 9\$.
  3. [optional step]
  4. Glue those random digits together.
  5. Present the digits to BigInt().
  6. Use rejection sampling to perhaps re-roll.

Using an "obvious" approach like this will make it much easier for future maintenance engineers to immediately see that the library routine satisfies its contract. An optional step might re-roll based on just the high order digit, to avoid some (expensive) BigInt() calls.

standard library

Consider calling getRandomValues to produce a bunch of Uint8Array values. Typically you'll win with a single call. Mask them down to four bits, then do rejection sampling of values greater than \$9\$.

magic numbers

The values \$1,000,000,000\$ and \$9\$ are not well motivated. At a minimum they deserve comments or helpful identifier names.

I understand we're talking about a value less than four billion (once I've inserted commas), but it's unclear how that value is related to \$49,007,199,254,740,991\$. Doesn't Math.random() offer 53 bits of entropy? Though this bit of documentation sounds like it could be Mersenne Twister, making me wonder why we would ever wish to call it:

Note: Math.random() does not provide cryptographically secure random numbers. Do not use them for anything related to security. Use the Web Crypto API instead

When an API says "random", that makes me think "indistinguishable", so I naturally gravitate toward crypto secure implementations.

one-liner

There's no point to compressing this pair of assignments down to a single line.

    var rand = [], digits = range. ...

Use another punch card! Put var digits on a line of its own.

cryptic type conversion

The | 0 hack is pointless. Be explicit; call Math.floor() already.

Similar critique for the Math.random() * 1000000000 | 0 expression. If there's some concern you have in mind, say it, don't be oblique about it.

BigInt is about decimal digits

The whole business of working in nine- or ten-digit chunks seems a little suspect. And there's no need for a BigInt() call within a tight loop.

  1. Determine how many digits are necessary; call it \$N\$.
  2. Loop to roll \$N\$ random digits, uniformly distributed from \$0 ~ .. \thinspace 9\$.
  3. [optional step]
  4. Glue those random digits together.
  5. Present the digits to BigInt().
  6. Use rejection sampling to perhaps re-roll.

Using an "obvious" approach like this will make it much easier for future maintenance engineers to immediately see that the library routine satisfies its contract. An optional step might re-roll based on just the high order digit, to avoid some (expensive) BigInt() calls.

standard library

Consider calling getRandomValues to produce a bunch of Uint8Array values. Typically you'll win with a single call. Mask them down to four bits, then do rejection sampling to discard values greater than \$9\$.

added 299 characters in body
Source Link
J_H
  • 43.3k
  • 3
  • 38
  • 158

magic numbers

The values \$1,000,000,000\$ and \$9\$ are not well motivated. At a minimum they deserve comments or helpful identifier names.

I understand we're talking about a value less than four billion (once I've inserted commas), but it's unclear how that value is related to \$49,007,199,254,740,991\$. Doesn't Math.random() offer 53 bits of entropy? Though this bit of documentation sounds like it could be Mersenne Twister, making me wonder why we would ever wish to call it:

Note: Math.random() does not provide cryptographically secure random numbers. Do not use them for anything related to security. Use the Web Crypto API instead

When an API says "random", that makes me think "indistinguishable", so I naturally gravitate toward crypto secure implementations.

one-liner

There's no point to compressing this pair of assignments down to a single line.

    var rand = [], digits = range. ...

Use another punch card! Put var digits on a line of its own.

cryptic type conversion

The | 0 hack is pointless. Be explicit; call Math.floor() already.

Similar critique for the Math.random() * 1000000000 | 0 expression. If there's some concern you have in mind, say it, don't be oblique about it.

BigInt is about decimal digits

The whole business of working in nine- or ten-digit chunks seems a little suspect. And there's no need for a BigInt() call within a tight loop.

  1. Determine how many digits are necessary; call it \$N\$.
  2. Loop to roll \$N-1\$\$N\$ random digits, uniformly distributed from \$0 ~ .. \thinspace 9\$.
  3. Roll the high order digit, perhaps the same way as before but with rejection sampling.[optional step]
  4. Glue those random digits together.
  5. Present the digits to BigInt().
  6. Use rejection sampling to perhaps re-roll.

Using an "obvious" approach like this will make it much easier for future maintenance engineers to immediately see that the library routine satisfies its contract. An optional step might re-roll based on just the high order digit, to avoid some (expensive) BigInt() calls.

standard library

Consider calling getRandomValues to produce a bunch of Uint8Array values. Typically you'll win with a single call. Mask them down to four bits, then do rejection sampling of values greater than \$9\$.

magic numbers

The values \$1,000,000,000\$ and \$9\$ are not well motivated. At a minimum they deserve comments or helpful identifier names.

I understand we're talking about a value less than four billion (once I've inserted commas), but it's unclear how that value is related to \$49,007,199,254,740,991\$. Doesn't Math.random() offer 53 bits of entropy? Though this bit of documentation sounds like it could be Mersenne Twister, making me wonder why we would ever wish to call it:

Note: Math.random() does not provide cryptographically secure random numbers. Do not use them for anything related to security. Use the Web Crypto API instead

When an API says "random", that makes me think "indistinguishable", so I naturally gravitate toward crypto secure implementations.

one-liner

There's no point to compressing this pair of assignments down to a single line.

    var rand = [], digits = range. ...

Use another punch card! Put var digits on a line of its own.

cryptic type conversion

The | 0 hack is pointless. Be explicit; call Math.floor() already.

Similar critique for the Math.random() * 1000000000 | 0 expression. If there's some concern you have in mind, say it, don't be oblique about it.

BigInt is about decimal digits

The whole business of working in nine- or ten-digit chunks seems a little suspect. And there's no need for a BigInt() call within a loop.

  1. Determine how many digits are necessary; call it \$N\$.
  2. Loop to roll \$N-1\$ random digits, uniformly distributed from \$0 ~ .. \thinspace 9\$.
  3. Roll the high order digit, perhaps the same way as before but with rejection sampling.
  4. Glue those random digits together.
  5. Present the digits to BigInt().

Using an "obvious" approach like this will make it much easier for future maintenance engineers to immediately see that the library routine satisfies its contract.

standard library

Consider calling getRandomValues to produce a bunch of Uint8Array values. Typically you'll win with a single call. Mask them down to four bits, then do rejection sampling.

magic numbers

The values \$1,000,000,000\$ and \$9\$ are not well motivated. At a minimum they deserve comments or helpful identifier names.

I understand we're talking about a value less than four billion (once I've inserted commas), but it's unclear how that value is related to \$49,007,199,254,740,991\$. Doesn't Math.random() offer 53 bits of entropy? Though this bit of documentation sounds like it could be Mersenne Twister, making me wonder why we would ever wish to call it:

Note: Math.random() does not provide cryptographically secure random numbers. Do not use them for anything related to security. Use the Web Crypto API instead

When an API says "random", that makes me think "indistinguishable", so I naturally gravitate toward crypto secure implementations.

one-liner

There's no point to compressing this pair of assignments down to a single line.

    var rand = [], digits = range. ...

Use another punch card! Put var digits on a line of its own.

cryptic type conversion

The | 0 hack is pointless. Be explicit; call Math.floor() already.

Similar critique for the Math.random() * 1000000000 | 0 expression. If there's some concern you have in mind, say it, don't be oblique about it.

BigInt is about decimal digits

The whole business of working in nine- or ten-digit chunks seems a little suspect. And there's no need for a BigInt() call within a tight loop.

  1. Determine how many digits are necessary; call it \$N\$.
  2. Loop to roll \$N\$ random digits, uniformly distributed from \$0 ~ .. \thinspace 9\$.
  3. [optional step]
  4. Glue those random digits together.
  5. Present the digits to BigInt().
  6. Use rejection sampling to perhaps re-roll.

Using an "obvious" approach like this will make it much easier for future maintenance engineers to immediately see that the library routine satisfies its contract. An optional step might re-roll based on just the high order digit, to avoid some (expensive) BigInt() calls.

standard library

Consider calling getRandomValues to produce a bunch of Uint8Array values. Typically you'll win with a single call. Mask them down to four bits, then do rejection sampling of values greater than \$9\$.

added 299 characters in body
Source Link
J_H
  • 43.3k
  • 3
  • 38
  • 158

magic numbers

The values \$1,000,000,000\$ and \$9\$ are not well motivated. At a minimum they deserve comments or helpful identifier names.

I understand we're talking about a value less than four billion (once I've inserted commas), but it's unclear how that value is related to \$49,007,199,254,740,991\$. Doesn't Math.random() offer 53 bits of entropy? Though this bit of documentation sounds like it could be Mersenne Twister, making me wonder why we would ever wish to call it:

Note: Math.random() does not provide cryptographically secure random numbers. Do not use them for anything related to security. Use the Web Crypto API instead

When an API says "random", that makes me think "indistinguishable", so I naturally gravitate toward crypto secure implementations.

one-liner

There's no point to compressing this pair of assignments down to a single line.

    var rand = [], digits = range. ...

Use another punch card! Put var digits on a line of its own.

cryptic type conversion

The | 0 hack is pointless. Be explicit; call Math.floor() already.

Similar critique for the Math.random() * 1000000000 | 0 expression. If there's some concern you have in mind, say it, don't be oblique about it.

BigInt is about decimal digits

The whole business of working in nine- or ten-digit chunks seems a little suspect. And there's no need for a BigInt() call within a loop.

  1. Determine how many digits are necessary; call it \$N\$.
  2. Loop to roll \$N-1\$ random digits, uniformly distributed from \$0 ~ .. \thinspace 9\$.
  3. Roll the high order digit, perhaps the same way as before but with rejection sampling.
  4. Glue those random digits together.
  5. Present the digits to BigInt().

Using an "obvious" approach like this will make it much easier for future maintenance engineers to immediately see that the library routine satisfies its contract.

standard library

Consider calling getRandomValues to produce a bunch of Uint8Array values. Typically you'll win with a single call. Mask them down to four bits, then do rejection sampling.

magic numbers

The values \$1,000,000,000\$ and \$9\$ are not well motivated. At a minimum they deserve comments or helpful identifier names.

I understand we're talking about a value less than four billion (once I've inserted commas), but it's unclear how that value is related to \$49,007,199,254,740,991\$.

one-liner

There's no point to compressing this pair of assignments down to a single line.

    var rand = [], digits = range. ...

Use another punch card! Put var digits on a line of its own.

cryptic type conversion

The | 0 hack is pointless. Be explicit; call Math.floor() already.

Similar critique for the Math.random() * 1000000000 | 0 expression. If there's some concern you have in mind, say it, don't be oblique about it.

BigInt is about decimal digits

The whole business of working in nine- or ten-digit chunks seems a little suspect. And there's no need for a BigInt() call within a loop.

  1. Determine how many digits are necessary; call it \$N\$.
  2. Loop to roll \$N-1\$ random digits, uniformly distributed from \$0 ~ .. \thinspace 9\$.
  3. Roll the high order digit, perhaps the same way as before but with rejection sampling.
  4. Glue those random digits together.
  5. Present the digits to BigInt().

Using an "obvious" approach like this will make it much easier for future maintenance engineers to immediately see that the library routine satisfies its contract.

standard library

Consider calling getRandomValues to produce a bunch of Uint8Array values. Typically you'll win with a single call. Mask them down to four bits, then do rejection sampling.

magic numbers

The values \$1,000,000,000\$ and \$9\$ are not well motivated. At a minimum they deserve comments or helpful identifier names.

I understand we're talking about a value less than four billion (once I've inserted commas), but it's unclear how that value is related to \$49,007,199,254,740,991\$. Doesn't Math.random() offer 53 bits of entropy? Though this bit of documentation sounds like it could be Mersenne Twister, making me wonder why we would ever wish to call it:

Note: Math.random() does not provide cryptographically secure random numbers. Do not use them for anything related to security. Use the Web Crypto API instead

When an API says "random", that makes me think "indistinguishable", so I naturally gravitate toward crypto secure implementations.

one-liner

There's no point to compressing this pair of assignments down to a single line.

    var rand = [], digits = range. ...

Use another punch card! Put var digits on a line of its own.

cryptic type conversion

The | 0 hack is pointless. Be explicit; call Math.floor() already.

Similar critique for the Math.random() * 1000000000 | 0 expression. If there's some concern you have in mind, say it, don't be oblique about it.

BigInt is about decimal digits

The whole business of working in nine- or ten-digit chunks seems a little suspect. And there's no need for a BigInt() call within a loop.

  1. Determine how many digits are necessary; call it \$N\$.
  2. Loop to roll \$N-1\$ random digits, uniformly distributed from \$0 ~ .. \thinspace 9\$.
  3. Roll the high order digit, perhaps the same way as before but with rejection sampling.
  4. Glue those random digits together.
  5. Present the digits to BigInt().

Using an "obvious" approach like this will make it much easier for future maintenance engineers to immediately see that the library routine satisfies its contract.

standard library

Consider calling getRandomValues to produce a bunch of Uint8Array values. Typically you'll win with a single call. Mask them down to four bits, then do rejection sampling.

Source Link
J_H
  • 43.3k
  • 3
  • 38
  • 158
Loading