dReKurCe has asked for the wisdom of the Perl Monks concerning the following question:

Greetings, I am somewhat at loss in formulating the appropriate concepulization of the following: The purpose of this script is to parce an arrray and total the numbers of hours and minutes. the field holding the data is a five elements long with the requitred field last. Heres my code: 
foreach $linex(@lines0){
         @lines=split(/ /,$linex);
         $time=$fields[4];
         @units=split(/:/,$time);
         $sec=$units[2];
         $min=$units[1];
         $hrs=$units[0];
         $tsec+=$sec;
         $tmin+=$min;
         $thrs+=$hrs;

if ($tmin>60){
    $rolloverm=$tmin%60;
    $tmin=$rolloverm;
    $thrs+=1;
}

if ($tsec>60){
   $rolloverm=$tmin%60;
   $thrs+=1;
}
print "Total time is $tmin minutes and $tsec seconds\n";

[download]
This produces the incorrect time...ant insight appreciated. Thanks...

Replies are listed 'Best First'.
Re: time accumulater
by bgreenlee (Friar) on Aug 06, 2004 at 16:29 UTC

    It would help if you provided sample data, but one problem I see with your algorithm is that if $tmin or $tsec are 120 or more, your rollovers won't work.

    You might also check out Time::Piece or Time::Local...although I'm guessing this is homework, and that would be cheating. :-)

    Update: actually, as long as the times are standard (i.e. you don't have something like 2:45:152), you can ignore what I said about the rollovers not working (although you should roll your seconds over before your minutets, as fergal suggested). But the big reason your code isn't working is that you're referencing $fields[4] when I suspect you mean $lines[4]. Of course, if you ran with -w or use warnings and use strict, you would have caught this yourself.

    Brad

[reply]
[d/l]
[select]
Re: time accumulater
by fergal (Chaplain) on Aug 06, 2004 at 16:29 UTC
    You haven't closed the } on your foreach so I'm assuming your indentation is correct and it should have been just before the first if. If that's right, then your problem is that you need to do 
    if ($tmin>60){
    $rolloverm=$tmin%60;
    $tmin=$rolloverm;
    $thrs+= int($tmin/60); # was 1
}

    [download]
    and similarly for $tsec.

    Also you should roll over your seconds before you rollover your minutes.

[reply]
[d/l]
Re: time accumulater
by CountZero (Bishop) on Aug 06, 2004 at 17:32 UTC
    From a conceptual point of view, I would "translate" all times into seconds, add the seconds and as the very last operation show the accumulated seconds as "days - hours - minutes - seconds" which is easily done with a cascade of "modulo" and "integer divisions".

    CountZero

    "If you have four groups working on a compiler, you'll get a 4-pass compiler." - Conway's Law

[reply]
Re: time accumulater
by VSarkiss (Monsignor) on Aug 06, 2004 at 17:41 UTC

    You have some good answers above, but I wanted to point out something. In this code: 

             @lines=split(/ /,$linex);
         $time=$fields[4];

    [download]
    you're setting @lines, and then looking for values in @fields. Unless that's just a copy-and-paste artifact, it could well be the source of your problem.

[reply]
[d/l]
[select]
Re: time accumulater
by ysth (Canon) on Aug 06, 2004 at 19:44 UTC
    Several problems: You should be checking >= 60, not > 60. Incrementing hours when the seconds total over 60 is not good, increment $tmin instead. And you do have to handle seconds overflow first, and then minutes, or you can be left with minutes of 60 or end up dropping an hour. I wouldn't bother with if blocks, just: 
    $tmin += $tsec >= 60;
$tsec %= 60;
$hrs += $tmin >= 60;
$tmin %= 60;

    [download]
[reply]
[d/l]
Re: time accumulater
by graff (Chancellor) on Aug 08, 2004 at 05:04 UTC
    So, each line of input has a space-separated token at the end that looks like "nn:nn:nn" (where each "n" is a digit), and this represents a duration, and you want to sum durations. Did I get that right?

    Are you confident that this final chunk of each line always has 2 colons and three sets of digits -- i.e. 10 seconds is sure to be rendered as "00:00:10" (or "0:00:10" or "0:0:10")?

    And do you really want the output to be expressed in minutes and seconds (even when the total may be several hours)?

    If you answered yes to all the above, then your code could be a lot shorter: 

    use strict;

my @lines0;  # using your array name

# ...do whatever you do to fill @lines0...

my $totalsec = 0;

for ( @lines0 ) {
    my @words = split /\s+/;
    my ( $hrs, $mins, $secs ) = split /:/, pop @words;
    $totalsec += ( $hrs * 60 + $mins ) * 60 + $secs;
}

printf( "Total time is %d minutes and %d seconds\n",
        int( $totalsec / 60 ), $totalsec % 60 );

    [download]
    In fact, you could eliminate the "@words" array in the for loop: 
    for ( @lines0 ) {
   my ( $hrs,$mins,$sec ) = split( /:/, pop( split /\s+/ ));
   $totalsec += ...
}

    [download]
    Now, if you have to deal with "deficient" strings (i.e. "00:10" or just "10"), you need to know how to resolve these, and adjust the code accordingly (or, if they occur and you don't know how to resolve them, you have to trap those problem lines somehow -- either die, or issue a warning about ignoring unusable input and keep going).

    If really large totals should be expressed in hours, minutes and seconds, I think it should be clear how to do that.

[reply]
[d/l]
[select]

Back to Seekers of Perl Wisdom