0
\$\begingroup\$

I have a PRNG-Algorithm:

/**
* Generates an integer. Min: 0 | Max: total - 1
* @param    total The number of possibilities.
* @return The generated number.*/
public function generate_normal(total:int):int
{
    _seed = (_seed * MULTIPLIER) % MODULUS;
    return (_seed * total / MODULUS);
}

The constants are defined as follows:

/**The Multiplier.*/
private const MULTIPLIER:int = 48271;

/**The Modulus.*/
private const MODULUS:int = 2147483647;

This is readable and understandable, but pretty slow (~50% more time).

Simply using the numbers would be faster, but less readable. Will people call these numbers "magic" and kill me or will everything be fine? Should I change this part in the class description:

Modulus suggested 1988 by Stephen K. Park and Keith W. Miller.
Multiplier suggested 1993 by Park and Miller.

to this:

Modulus (2147483647) suggested 1988 by Stephen K. Park and Keith W. Miller.
Multiplier (48271) suggested 1993 by Park and Miller.

Should I keep my approach or use the numbers directly?

TL;DR What's more important, performance or readability?

\$\endgroup\$
1
\$\begingroup\$

There is no one answer.

Performance

How often is generate_normal called? Is it called on a program critical path or only during set up?

Could you cache a bunch of numbers before hand and only replenish the cache when you run out?

Readability

How many people will read this code over its lifetime? How often will you have to come back to the code?

Compromise

If you notice a substantial penalty using the private const construct, then do the following:

/**The Multiplier. Suggested 1993 by Park and Miller.*/
private const MULTIPLIER:int = 48271; // folded into code below

/**The Modulus. Suggested 1988 by Stephen K. Park and Keith W. Miller.*/
private const MODULUS:int = 2147483647; // folded into code below

...

public function generate_normal(total:int):int
{
    // Values folded from definitions ...
    // _seed = (_seed * MULTIPLIER) % MODULUS;
    _seed = (_seed * 48271) % 2147483647;
    // return (_seed * total / MODULUS);
    return (_seed * total / 2147483647);
}
\$\endgroup\$
1
  • \$\begingroup\$ Performance: I'll be using it mostly for games, about 5 times per frame; nice idea with the cache. Readability: I think that at least I'll be sharing it with some people who help me with the games. \$\endgroup\$
    – user42193
    May 10 '14 at 14:35
0
\$\begingroup\$

Readability:

>>> hex(2147483647)
'0x7fffffff'

Using a hex constant would be way more readable.

Performance:

Honestly I can't believe that using the numbers would be faster. Do you have any measurements to support the claim?

\$\endgroup\$
2
  • \$\begingroup\$ I just made a little test: I stored the current time in milliseconds, tested with constants (25.000.000 iterations) and stored the current time minus the time before. Then I did the same again without constants. The times varied alot, but this was one run: With constants: 3398 | Without constants: 2211 | With constants took 53% more time. \$\endgroup\$
    – user42193
    May 12 '14 at 12:04
  • \$\begingroup\$ there's a small increase (7-9%) but i can't see anything close to 53%. maybe there's some complex scope that makes it difficult to go read those values every time \$\endgroup\$
    – alebianco
    Sep 5 '16 at 15:25

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy