tangent.gardner has asked for the wisdom of the Perl Monks concerning the following question:

for (;;) { foreach my $item (sort (keys %timings)) { do_stuff(); # changes data, not keys, no deletes } }
This works ok for a few iterations of the outer loop, but after about 200, the foreach seems to ignore all but a couple of %timings members.

Replies are listed 'Best First'.
Re: looping through hash of hashes
by Corion (Patriarch) on Dec 03, 2015 at 12:28 UTC

    Most likely, %timings does not contain what you think it does.

    Maybe inspect it before every iteration:

    use Data::Dumper; ... for (;;) { warn "Next iteration:"; warn Dumper \%timings; foreach my $item (sort (keys %timings)) { warn "Processing [$item]"; do_stuff(); # changes data, not keys, no deletes } }
      Nah, I had already tried that. Dumper shows that some of %timings's key values do not get changed. I am puzzled, because I am treating them all the same.
      #!/usr/bin/perl use strict; use Data::Dumper; #Wed Dec 02 06:22:50 GMT 2015: changed handling of DEBUG #Mon Nov 30 21:17:12 GMT 2015: done away with status array. Every thi +ng now stored in %timings hash my %status; my $COMPLETE = 0; my $MAX_SLEEP = 5; my $MIN_LOG_DELAY = 6; #seconds my $LOG_DELAY; my $timings_file = shift; my $DEBUG = 0; my $TRUE = 1; my $FALSE = 0; my $ON = $TRUE; my $FINISHED = 2; my $STARTED = 1; my $elapsed_seconds = 0; my $now; my %timings; set_timings(); open(LOG, ">control.log") || die "cannot open control.log"; ## START ## $elapsed_seconds = time - $^T; $now = $elapsed_seconds; for(;;) { my $item; foreach $item (keys %timings) { warn "Next iteration:"; warn Dumper \%timings; warn "Processing [$item]"; check($item); } $elapsed_seconds = time - $^T; if (($elapsed_seconds - $now) > $LOG_DELAY) { show_status(); $now = $elapsed_seconds; } $COMPLETE = 1; foreach $item (keys %timings) { if ($timings{$item}{STATUS} != '2') { $COMPLETE = 0; last; } } if ($COMPLETE) { show_status(); die "finished\n" } } sub set_timings() { open(IN, $timings_file) || die "cannot open $timings_file\n"; while (!eof(IN) ) { $_ = <IN>; $DEBUG = 1 if ($_ =~ m/^DEBUG/); if ($_ > 0) { $LOG_DELAY = $_; warn "log delay is $LOG_DELAY. This is very low and you risk +running out of disk space!\n" if $LOG_DELAY < $MIN_LOG_DELAY; next; } next if $_ =~ m/^#/; next unless s/^(.*?):\s*//; my $item = $1; for my $field ( split ) { (my $key, my $value) = split /=/, $field; $timings{$item}{$key} = $value; } die "$item start: $timings{$item}{START} is less than stop:$timing +s{$item}{STOP}\n" if $timings{$item}{START} > $timings{$item}{STOP}; } } sub check(my $item) { my $item = shift; show_status() if ($DEBUG); $elapsed_seconds = time - $^T; if (($elapsed_seconds >= $timings{$item}{START}) and ($timings{$it +em}{STATUS} != '2')) { switch_on($item); $timings{$item}{STATUS} = '1'; } if ($elapsed_seconds > $timings{$item}{STOP}) { last if $timings{$item}{STATUS} == '2'; switch_off($item); $timings{$item}{STATUS} = '2'; # keep it off } } sub switch_on(my $item, my $addr) { my $item = shift; if ($timings{$item}{STATUS} == '1') { #already on return(0); } if ($timings{$item}{STATUS} == '2') { return(0); } # do print on stuff } sub switch_off(my $item, my $addr) { my $item = shift; if ($timings{$item}{STATUS} == '2') { return(0); } # do switch off stuff $timings{$item}{STATUS} = '2'; } sub show_status() { foreach my $item (keys %timings) { if ($DEBUG) { print "$item, $elapsed_seconds, $timings{$item}{START}, $timin +gs{$item}{STOP}, $timings{$item}{STATUS}\n"; } else { select((select(LOG), $|=1)[0]); # flush the log file print LOG "$item, $elapsed_seconds, $timings{$item}{START}, $t +imings{$item}{STOP}, $timings{$item}{STATUS}\n"; } } print LOG "##################\n"; }
      And here is the data file:
      RED: ADDR=1 START=1 STOP=2 STATUS=0 BLUE: ADDR=2 START=1 STOP=2 STATUS=0 GREEN: ADDR=3 START=1 STOP=2 STATUS=0 YELLOW: ADDR=4 START=1 STOP=2 STATUS=0 VIDEO: ADDR=5 START=1 STOP=2 STATUS=0 MOTOR_FORWARD: ADDR=1 START=1 STOP=2 STATUS=0 MOTOR_REVERSE: ADDR=27 START=1 STOP=2 STATUS=0 AUDIO: ADDR=8 START=1 STOP=10 STATUS=0 SPARE: ADDR=9 START=1 STOP=2 STATUS=0
      My original assumption about the number of outer iterations is wrong. If the STOP vales are all the same everything works fine. Things go funny if they are different. Obviously there is something wrong with my logic, or my understanding of hashes.
        Of course some of the items are not processed. You exit the loop by last. You don't use warnings, why? They would have told you:
        Exiting subroutine via last at ./1.pl line 91, <IN> line 10.
        ($q=q:Sq=~/;[c](.)(.)/;chr(-||-|5+lengthSq)`"S|oS2"`map{chr |+ord }map{substrSq`S_+|`|}3E|-|`7**2-3:)=~y+S|`+$1,++print+eval$q,q,a,
Re: looping through hash of hashes
by Athanasius (Archbishop) on Dec 05, 2015 at 03:38 UTC

    Hello tangent.gardner, and welcome to the Monastery!

    choroba++ has identified the logic error in your script. I just want to point out a few aspects of your code which could be improved.

    1. Perl is not C, and Perl’s function prototypes work quite differently. See perlsub#Prototypes. As a rule, it’s best to dispense with them altogether, unless you (a) have a particular use case which requires prototypes and (b) know how to use them correctly. See Far More than Everything You've Ever Wanted to Know about Prototypes in Perl -- by Tom Christiansen.

    2. This snippet of code:

      while (!eof(IN) ) { $_ = <IN>; $DEBUG = 1 if ($_ =~ m/^DEBUG/)

      can be written more idiomatically like this:

      while (<IN>) { $DEBUG = 1 if /^DEBUG/;
    3. The test if ($_ > 0) { which immediately follows the previous snippet throws warnings whenever $_ is not a number. You should add an additional test; for example:

      use Scalar::Util qw( looks_like_number ); ... if (looks_like_number($_) && ($_ > 0)) {
    4. In a conditional test, the type of the operands should match the type of the operator. The numeric comparison operators are == != < <= > >= <=>, and their string counterparts are eq ne lt le gt ge cmp (see perlop#Equality Operators). So, e.g., this line:

      if ($timings{$item}{STATUS} != '2') {

      would be better written either as:

      if ($timings{$item}{STATUS} ne '2') {

      (if the hash value is stored as a string), or as:

      if ($timings{$item}{STATUS} != 2) {

      (if the hash value is stored as a number). Yes, Perl will generally perform a silent string-to-number or number-to-string conversion where required, but it’s better practice to make your code self-documenting by using operands and operators of matching types.

    5. die should be used only for exceptions, not for normal program termination. The final lines of your main code would be better written as follows:

      for (;;) { ... if ($COMPLETE) { show_status(); print "finished\n"; last; } } close $log or die "Cannot close file '$log_file': $!";

    Hope that helps,

    Athanasius <°(((><contra mundum Iustus alius egestas vitae, eros Piratica,

Re: looping through hash of hashes
by AnomalousMonk (Archbishop) on Dec 05, 2015 at 16:11 UTC

    Further to choroba's point about warnings and Athanasius's point 1 about prototypes (++both): Using warnings would have alerted you the fact that there was more than one unkosher thing about the way you were using prototypes. Without warnings:

    c:\@Work\Perl\monks>perl -le "use strict; ;; foo(); sub foo (my $x) { my $x = 'hiya'; print $x; } " hiya
    And with:
    c:\@Work\Perl\monks>perl -le "use strict; use warnings; ;; foo(); sub foo (my $x) { my $x = 'hiya'; print $x; } " Illegal character in prototype for main::foo : my$x at -e line 1. main::foo() called too early to check prototype at -e line 1. hiya


    Give a man a fish:  <%-{-{-{-<

      Thanks a lot. My current and future code will include all these things. Thanks again. Joe.