1. INIT blocks are broken in Perl 5. Use BEGIN blocks instead.
I need the INIT block to initialize the PRNG after the user's seed source choices are known from the import subroutine. There's no way around it.

I added a cautionary note and sample code to work around this issue if the user wants to require my module.

2. I understand the desire to make a drop-in replacement for Perl's rand (and srand), but I think it is a huge missed opportunity not to make this PRNG available as objects. PRNGs are a special kind of iterator; most, if not all, of the benefits of iterator objects apply to PRNG objects. Specifically, the possibility of having multiple independent PRNGs operating in parallel enables much greater modularization than having a single global PRNG like rand. This can be a real time saver, specially during testing and debugging.
It's an engineering decision. I opted for speed and simplicity.

Math::Random::MT has an OO interface for the purposes you describe. The only significant feature it's missing is the ability to save and restore PRNG state.

UPDATE: I have now added a complete OO interface that is still faster than Math::Random::MT, and at the same time maintained the speed improvement on the standalone PRNG. Support for saving and restoring state vectors was also added.

3. I agree that statistical tests are essential for the this module's test suite.
Statistical testing would flush out two possibilities: Either the algorithm is bad, or the code is incorrect.

As I mentioned before, there seems to be adequate analysis of the algorithm to show that it is fine.

As for the code, I compare the output of the PRNG for a known seed against known output provided by the algorithm's authors. (The test uses 2000 random number which exercises the shuffle routines through 4 cycles.) If that test passes, then the code is definitely correct. (Such a deterministic test is, in fact, much more stringent than a statistical one.)

4. I also agree with the opinion that _seed should be broken up. I would use a dispatch table...
I have broken up this subroutine. I'll use a dispatch table as well.
5. PRNG code often uses one or several "magic numbers", like the 624 that appears frequently in the implementation of _seed. As shown in the code given in the previous point, it is both easier to read and to maintain if one puts these into file-scoped variables or constants or accessors. (My preference is to use lexicals with corresponding getters or getter/setters, depending on the desired API.)
I put in a lexical variable.
6. I am confused by this bit in import 
        elsif ($sym =~ 'auto') {
            # To auto-seed or not
            $auto_seed = $sym =~ /no|!/;
        }

[download]
Eeek! A bug! (See my tag line. ;) Thanks. I'll squash it.
7. It might be nice to have access (e.g. through a getter) to the user agent object used when contacting remote sources over HTTP. This would be similar to what LWP::Simple provides, but using an accessor, instead of exporting the package variable.
As far as I can see, there does not seem to be any need for the user to have access to the user agent object, and after a seed is acquired, there is no need to keep it around. I guess I don't understand your point.
8. IMO the documentation could do a better job in explaining when someone may want to use this module over Perl's standard rand, or vice-versa.
I added a bit of this to the POD.
9. I think one should relax strict for the smallest scope that requires it. Hence, I'd move the 
no strict 'refs'<code> to
<code>
        { no strict 'refs'; *{"${pkg}::$sym"} = \&$sym; }

[download]
Done.
10. require doesn't import 
            eval { require LWP::UserAgent; };
            if ($@) {
                push(@errors, "Failure loading LWP::UserAgent: $@");
                next;
            }

[download]
I think you meant for me to add an import call. I will.
Actually, I prefer this idiom: 
            eval { require LWP::UserAgent }
              or do {
                      push @errors,
                        "Failure loading LWP::UserAgent: $@";
                      next;
                    };

[download]
Not my style, but then again TMTOWTDI. :)

Thanks for the great feedback. I'll upload my changes to CPAN soon.


Remember: There's always one more bug.

In reply to Re^2: New CPAN Module: Math::Random::MT::Auto by jdhedden
in thread New CPAN Module: Math::Random::MT::Auto by jdhedden

Title:
Use:  <p> text here (a paragraph) </p>
and:  <code> code here </code>
to format your post, it's "PerlMonks-approved HTML":


  • Posts are HTML formatted. Put <p> </p> tags around your paragraphs. Put <code> </code> tags around your code and data!
  • Titles consisting of a single word are discouraged, and in most cases are disallowed outright.
  • Read Where should I post X? if you're not absolutely sure you're posting in the right place.
  • Please read these before you post! —
  • Posts may use any of the Perl Monks Approved HTML tags:
    a, abbr, b, big, blockquote, br, caption, center, col, colgroup, dd, del, details, div, dl, dt, em, font, h1, h2, h3, h4, h5, h6, hr, i, ins, li, ol, p, pre, readmore, small, span, spoiler, strike, strong, sub, summary, sup, table, tbody, td, tfoot, th, thead, tr, tt, u, ul, wbr
  • You may need to use entities for some characters, as follows. (Exception: Within code tags, you can put the characters literally.)
            For:     Use:
    & &amp;
    < &lt;
    > &gt;
    [ &#91;
    ] &#93;
  • Link using PerlMonks shortcuts! What shortcuts can I use for linking?
  • See Writeup Formatting Tips and other pages linked from there for more info.