<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 <u...@stemsystems.com>
To: beginners@perl.org
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, jmrhide-p...@yahoo.com 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     => 'i...@theinfosite.com',
>      To         => 'jmrhide-theinfos...@yahoo.com',
>      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: beginners-unsubscr...@perl.org
For additional commands, e-mail: beginners-h...@perl.org
http://learn.perl.org/

Reply via email to