<why is that sub being declared in BEGIN?>
I copied what was on the manpage for CGI::carp
<for (my $loopiter <= $loopmax)>
I lost my way on that one, but I don't see how a shuffle works here either. I'm
trying to ensure that the program will not pick the same term twice in a row.
$lastnum comes from a cookie and refers to the previous question. What about
for (my $loopiter = 1..$loopmax) { #1
if $loopiter >= $loopmax { die "Loop number $loopnum has iterated
$loopiter
times" }
$termnum[0] = int( rand($defnum) );
last unless ( $termnum[0] == $lastnum );
}
Assuming 25 to 50 terms in a category (the range for $defnum), there's a small
but finite chance that this randomly chosen $termnum will be exactly the same
as
the previous one ($lastnum). The chance is even smaller that it will happen
that
way twice in a row. So this loop should not have many iterations and it's hard
for me to see how it could go wrong. Some of the later loops have more
complicated conditions: for example, the new $termnum must not match any of
several previously chosen others. I see more room for that sort of loop to go
awry. So I'm just trying to build in some insurance against logical errors
specific to these looping structures that recur throughout the process of
constructing new questions
John M Rathbun MD
________________________________
From: Uri Guttman <[email protected]>
To: [email protected]
Sent: Sun, September 16, 2012 8:10:50 PM
Subject: Re: Fw: Proposed correction for my long script
On 09/16/2012 07:40 PM, [email protected] wrote:
> I guess I've been vague again. I was really hoping for somebody to cast an eye
> on the subroutines I added earlier today. My intention was to (1) prevent any
> infinite looping and (2) get notification if one of the loops misbehaves. Did
I
> make any obvious logical or grammatical errors in the snippets of code below?
>
to be nice, there are too many logical errors to even count. somehow you got
this working but redoing it to reasonable code will be a major project.
> #!/usr/local/bin/perl
>
> use warnings;
> use strict;
> use diagnostics;
>
you keep being told to drop diagnostics. why haven't you?
> use CGI::carp qw(set_die_handler);
> BEGIN {
> sub handle_errors {
> my $msg = shift;
> sendmail(
> From => '[email protected]',
> To => '[email protected]',
> Subject => 'Tutorial Feedback',
> Message => $msg,
> );
> }
> set_die_handler(\&handle_errors);
> }
it that is the formatting of perltidy, you should still improve it. arguments
to
subs should be indented as should code lines of subs. that looks like one long
list of things but it is several nested ones. why is that sub being declared
in
BEGIN? all subs are compiled at compile time so that should make no difference.
>
>
>
> if ( $answer or $restart ) {
> if ( $playlevel < 1 ) {
> for (my $loopiter <= $loopmax) { #1
that is not how to write a for loop. for loops take a list of values. that is
one value so this will execute exactly one time regardless of the boolean value
(which will be aliased to $_). you likely mean while there but with the if
below
checking the same thing (but inverted comparison) i can't follow what you want
here.
> if $loopiter >= $loopmax { die "Loop number $loopnum has
> iterated $loopiter times" }
> $termnum[0] = int( rand($defnum) );
> last unless ( $termnum[0] == $lastnum );
as others have stated you likely want a shuffle which will always terminate.
you
checking for duplicated random values is silly. it may be possible for an
infinite (or very long) loop. it is very poor logic.
> Does it look OK?
>
no. it needs a cat scan, mri, organ transplants, mani/pedi, and a good steak
dinner! :)
uri
-- To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
http://learn.perl.org/
