www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.bugs - [Issue 10434] New: Don't use Random as template parameter name in std.random (or anywhere else)

reply d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10434

           Summary: Don't use Random as template parameter name in
                    std.random (or anywhere else)
           Product: D
           Version: D2
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: normal
          Priority: P2
         Component: Phobos
        AssignedTo: nobody puremagic.com
        ReportedBy: joseph.wakeling webdrake.net


--- Comment #0 from Joseph Rushton Wakeling <joseph.wakeling webdrake.net>
2013-06-20 16:18:36 PDT ---
Quite a number of functions and objects in std.random use Random as the
template parameter name for a random number generator -- e.g. in RandomCover:

    struct RandomCover(Range, Random)
        if(isRandomAccessRange!Range && isUniformRNG!Random)

This is obviously dangerous as it risks accidental confusion with the alias
Random used for the default RNG type.

Elsewhere, the very long UniformRandomNumberGenerator is used as a typical
parameter name -- descriptive but a pain to retype!

So, I suggest replacing these parameter name with something consistent and more
appropriate -- either Rng (which is used in some other places in std.random) or
RNG (which I prefer to Rng, but which maybe isn't in line with D style:-)

If there's a desire to stress that a parameter is a _uniform_ random number
generator, we could also use Urng or URNG or even UniformRNG, but I think
that's overkill as those features of the parameter should be indicated by
template constraints, not naming conventions that are bordering on the
Hungarian.

We could also take this opportunity to standardize the typical variable name
for a RNG -- which in some places is called rnd, in some rng, in some urng, and
in some gen.

So, I propose standardizing on RNG or Rng for template parameters (someone tell
me which:-) and rng for variable names.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jun 20 2013
next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10434


Andrej Mitrovic <andrej.mitrovich gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |andrej.mitrovich gmail.com


--- Comment #1 from Andrej Mitrovic <andrej.mitrovich gmail.com> 2013-06-20
16:23:40 PDT ---
RandGen could also work.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jun 20 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10434


hsteoh quickfur.ath.cx changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hsteoh quickfur.ath.cx


--- Comment #2 from hsteoh quickfur.ath.cx 2013-07-15 16:16:22 PDT ---
+1 for RandGen, self-documenting and doesn't clash with existing symbol. Rng is
a bit obscure for those not in the know.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jul 15 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10434



--- Comment #3 from github-bugzilla puremagic.com 2013-08-29 00:34:59 PDT ---
Commits pushed to master at https://github.com/D-Programming-Language/phobos

https://github.com/D-Programming-Language/phobos/commit/8d9233cf8b9e4d27bd70dd0fcd171d2f6dc2f2c0
Partial fix for Issues 7067 and 10434 - std.random.RandomCover

The existing RandomCover design is fatally flawed because it
requires a RNG as input, which it then copies internally by
value.  So, unless the user is smart enough to pass something
like e.g. SomeRNG(unpredictableSeed), there will be unintended
correlations in random behaviour.

This partial fix follows the design of RandomSample in allowing
RandomCover to use the thread-global default RNG rndGen.  It
also improves the choice of template parameter and variable
names in line with Issue 10434.

https://github.com/D-Programming-Language/phobos/commit/2c9ecb8bd146a77c5ca450a69771b0d9dfcdf732
Partial fix for Issue 10434 - std.random.RandomSample

In line with changes to RandomCover, this patch tweaks
the choice of template parameter and variable names in
order to bring clarity and uniformity to the module.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Aug 29 2013
prev sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10434



--- Comment #4 from Joseph Rushton Wakeling <joseph.wakeling webdrake.net>
2013-08-30 03:02:56 PDT ---
(In reply to comment #2)
 +1 for RandGen, self-documenting and doesn't clash with existing symbol. Rng is
 a bit obscure for those not in the know.
If you look at the pull which has just landed, I settled on UniformRNG in order to match the isUniformRNG template, with RandomGen as a secondary choice. I decided this was preferable since the uniformity of the generator _does_ matter. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
Aug 30 2013