<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/