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

hello, monks! it's been a little while, and i need some help with a small thing. i've figured out how to make this entire thing run, but i can't figure out how to clear the foreach loop for every new set of data it uploads from a file. the explicit package name for "@array" can't be moved into the loop because the subroutine below needs it as well, which seemed to be what everyone said would fix the problem.

is there any other way to clear the data each time? i've tried 'next', 'last', and 'redo', and none of them seem to do anything besides mess the loop up entirely. i've also tried adding another array into the loop using the original array to manipulate the data into clearing that way, but as i thought, that did nothing.

does anyone have any simple way of getting the data to clear for each loop? i'm sure it's something simple that i'm just overlooking. here's the code itself:

#!usr/bin/perl

use strict;
use diagnostics; 
use warnings;

my @array;
my $average;

 while (1) {
    print "Enter an input file name: ";
    chomp (my $choice = <STDIN>);
    open (FH, '<', "C:/Users/tsukk/$choice");

    while (my $data = <FH>) {
        push @array, $data;
    }
    
    my @largest = sort {$b<=>$a} @array;
    my @smallest = sort {$a<=>$b} @array;
    
    print "\nThe largest number is: "; 
    print $largest[0], "\n";
    print "The smallest number is: "; 
    print $smallest[0];
    print "The average of the numbers is: ";
    printf "%.1f\n", average(@array);
    print "\n";

    $average = average(@array);
    
    foreach (@array) {
        chomp;
        if ($average > $_) {
        print "$_\t is below average.\n";
        } elsif ($average < $_) {
            print "$_\t is above average.\n";
        } elsif ($average = $_) {
            print "$_\t is equal to average.\n";
        }
    }
    
    print "\nDo it again? (Yes or No): ";
    chomp (my $yesno=<STDIN>);
    
    if($yesno ne 'Yes') {
        print "Goodbye.\n";
        exit;
    } 
}
 
sub average {
if (@array) {
    my @temp = @_;
    my $sum = 0;
    foreach (@temp) {
        $sum = $sum + $_;
    }
    return $sum/@temp;
    }
}

[download]

thank you!

Replies are listed 'Best First'.
Re: resetting a foreach loop!
by pryrt (Abbot) on Nov 16, 2017 at 22:13 UTC

    Change average() to use its parameters, not the top-level @array directly; then you can limit @array to the appropriate context:

    sub average {
    if (@_) {
        my @temp = @_;
        my $sum = 0;
        foreach (@temp) {
            $sum = $sum + $_;
        }
        return $sum/@temp;
    }
}

    [download]

    edit: fixed indenting for readability

[reply]
[d/l]
[select]

      Your average specifically checks if no arguments are provided, but doesn't actually handle that situation. Fixed: 

      sub average {
    if (@_) {
        my $sum;
        for (@_) {
            $sum += $_;
        }
        return $sum/@_;
    } else {
        return undef;
    }
}

      [download]

      Same, but shorter: 

      sub sum { my $sum; $sum += $_ for @_; $sum }
sub avg { @_ ? sum(@_)/@_ : undef }

      [download]
[reply]
[d/l]
[select]

        ++Indeed. Technically, the no-explicit-return version of the function returns 0 for an empty argument list, so it implictly handles the empty case. (I infer that the OP did the if(@_) just to prevent divide-by-zero.) But I agree that explicit undef is a better return value than relying on a default 0.

[reply]
[d/l]
      that fixed everything, thank you so much!!
[reply]
        Sorting the same array twice and assigning the results to two new arrays, potentially very long, is a bit wasteful when all you want is to find the smallest and largest value in the array. You could for instance do something like this:

         my($largest, $smallest) = (sort { $b <=> $a } @array)[0,-1];

[reply]
[d/l]
Re: resetting a foreach loop!
by AnomalousMonk (Archbishop) on Nov 16, 2017 at 23:03 UTC
    ... i can't figure out how to clear the foreach loop for every new set of data it uploads from a file.

    The push built-in only adds elements to the end of an array (as you have discovered). Somewhere near the top of the main
        while (1) { ... }
    loop and before reading the file into the array, add a
        @array = ();
    statement to clear the array.

    Better yet, IMHO: Because the  @array array is not used outside the while-loop in this code, only declare the array at the point at which it is needed. (The same applies to the  $average scalar.) Something like 

    my @array;

while (my $data = <FH>) {
    push @array, $data;
}

    [download]
    or more simply, forget the while-loop and just 
    my @array = <FH>;

    [download]
    Combined with the change to the  average() function that pryrt suggested, things should go more smoothly.

    Update: Note that in the 

    foreach (@array) {
    chomp;
    if ($average > $_) {
    print "$_\t is below average.\n";
    } elsif ($average < $_) {
        print "$_\t is above average.\n";
    } elsif ($average = $_) {
        print "$_\t is equal to average.\n";
    }
}

    [download]
    loop of the OPed code, the  elsif ($average = $_) { ... } clause of the ladder of if-tests does not work as I expect you expect. The  $average = $_ expression assigns to  $average and does not test it. (Update: An assignment of 0 evaluates as 0, which is boolean false.) The result is that for an element of 0 in the array, the body of that clause is not executed (update: because an assignment of 0 evaluates to 0, which is boolean false): 
    c:\@Work\Perl\monks>perl -wMstrict -le
"my @array = (-2, -1, 0, 1, 2);
 my $average = 0;
 ;;
 foreach (@array) {
   if ($average > $_) {
   print qq{$_\t is below average.};
   } elsif ($average < $_) {
       print qq{$_\t is above average.};
   } elsif ($average = $_) {
       print qq{$_\t is equal to average.};
   }
 }
"
-2       is below average.
-1       is below average.
1        is above average.
2        is above average.

    [download]
    I assume you meant  $average == $_ as the test, although at that point the variables can only be equal (i.e., neither is greater nor less than the other), so a plain
        else {
            print "... equal to ...";
            }
    would have been sufficient.

    And another thing...
    The code posted by pryrt for the  average() subroutine returns 0 for an empty list of numbers passed to the function. 

    c:\@Work\Perl\monks>perl -wMstrict -le
"print average();
 ;;
 sub average {
     if (@_) {
         my @temp = @_;
         my $sum = 0;
         foreach (@temp) {
             $sum = $sum + $_;
         }
         return $sum/@temp;
     }
 }
"
0

    [download]
    I think I would have elected to return undef or perhaps throw an exception, but this behavior is the option of the programmer. However, one should be aware of it.

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

[reply]
[d/l]
[select]
Re: resetting a foreach loop!
by Laurent_R (Canon) on Nov 16, 2017 at 23:19 UTC
    Hi lunette,

    pryrt gave you the solution, but there is one point to be noted and that you need to understand.

    In general, when you write a subroutine, it is usually best if the subroutine can work only on parameters passed to it, and not on global variables.

    In your code, you pass the data to the subroutine as an argument: 

    $average = average(@array);

    [download]
    but you don't use the parameter within the average subroutine, but use the global array variable. In other words, you made half of the work needed, but not the whole thing. Here you might want to do something like this: 
    sub average {
    my @local_array = @_;
    if (@local_array) {
# ...
}

    [download]
    There are some exceptions, sometimes (rarely), there is a good reason to work directly on a global array or hash within a subroutine (for example for performance reasons if the array or hash is really very large), but, generally speaking, it is most of the time better for subroutines to work on copies of arguments passed to them.
[reply]
[d/l]
[select]
Re: resetting a foreach loop!
by pryrt (Abbot) on Nov 17, 2017 at 14:43 UTC

    By the way, I noticed a couple more things:

    • Use lexical filehandles: open my $fh, ... instead of open FH, ...
    • Check for errors: open my $fh, ...   or die "...$!", or use autodie at the beginning of your script
    • You had a direct call to average(@array) in the print, and another call with the same data when assigning to the variable $average = average(@array); no need to average the same data twice.

    To bring all the excellent suggestions to date into one post, plus these new suggestions, here is an annotated version of the updated code (note I changed to c:\temp\tsukk instead of c:\users\tsukk)

    use strict;
use diagnostics;
use warnings;

while (1) {
    print "Enter an input file name: ";
    chomp (my $choice = <STDIN>);
    open (my $fh, '<', "C:/temp/tsukk/$choice")                     # 
+  [pryrt] use lexical filehandles
        or die "error opening $choice: $!";                         # 
+  [pryrt] check for errors; if you don't want this, "use autodie;" ab
+ove

    # my @array; while (my $data = <$fh>) { push @array, $data; }   # 
+  [AnomalousMonk]'s [id://1203631] 'better yet': bring my @array into
+ the while(1)...
    my @array = <$fh>;                                              # 
+  [AnomalousMonk]'s [id://1203631] 'simply': slurp the whole file by 
+giving <> list context

    my $use_brostad = 1;                                            # 
+  set to 1 to use [brostad]'s, else false to use the anonymous sugges
+tion
    my ($largest, $smallest) = (-9e9,9e9);                          # 
+  initialize to the wrong extremes
    if($use_brostad) {
        ($largest, $smallest) = (sort {$b <=> $a } @array)[0,-1];   # 
+  [brostad]'s [id://1203655]: reverse sort, return the first and last
+ elements
    } else {
        for my $v (@array) {                                        # 
+  [anonymous monk]'s [id://1203660]: single pass through loop, withou
+t sorting; more efficient than brostad's single sort
            $largest = $v   if $v > $largest;
            $smallest = $v  if $v < $smallest;
        }
    }

    print "\nThe largest number is: ";
    print $largest, "\n";                                           # 
+  [pryrt]: largest is now scalar, not array
    print "The smallest number is: ";
    print $smallest;                                                # 
+  [pryrt]: smallest is now scalar, not array
    print "The average of the numbers is: ";
    printf "%.1f\n", my $average = average(@array);                 # 
+  [pryrt]: only call average() once per dataset
    print "\n";

    foreach (@array) {
        chomp;
        if ($average > $_) {
            print "$_\t is below average.\n";
        } elsif ($average < $_) {
            print "$_\t is above average.\n";
        } elsif ($average == $_) {                                  # 
+  [AnomalousMonk]'s [id://1203631]'s fix: comparison, not assigment
            print "$_\t is equal to average.\n";
        }
    }

    print "\nDo it again? (Yes or No): ";
    chomp (my $yesno=<STDIN>);

    if($yesno ne 'Yes') {
        print "Goodbye.\n";
        exit;
    }
}

sub average {
    if (@_) {                                                       # 
+  [pryrt]'s [id://1203627]
        my @temp = @_;
        my $sum = 0;
        foreach (@temp) {
            $sum = $sum + $_;
        }
        return $sum/@temp;
    }
}

    [download]
[reply]
[d/l]
[select]
      my ($largest, $smallest) = (-9e9,9e9);                          #   in
+itialize to the wrong extremes
    ...
        for my $v (@array) {                                    #   [a
+nonymous monk]'s [id://1203660]: single pass through loop, without so
+rting; more efficient than brostad's single sort
            $largest = $v   if $v > $largest;
            $smallest = $v  if $v < $smallest;
        }

      [download]

      The only quarrel I have with this implementation is that it depends on assumptions about the smallest and largest representable numbers (the wrongest extremes) in the system. Even if the assumptions are true in a given system, all bets are off if you move, e.g., to a different platform: from a 32-bit float to a 64-, 80- or who-knows-how-many-bit float. And if they're not true: 

      c:\@Work\Perl\monks>perl -wMstrict -le
"my @array = (-9e9-123, -9e9-234);
 ;;
 my ($largest, $smallest) = (-9e9, 9e9);
 ;;
 for my $elem (@array) {
   $largest  = $elem if $elem > $largest;
   $smallest = $elem if $elem < $smallest;
   }
 print qq{smallest: $smallest;  largest: $largest};
"
smallest: -9000000234;  largest: -9000000000

      [download]
      Taking the initial smallest/largest value from the array itself is bulletproof: either the initializer is already the smallest/largest value, or some other value will be found in the array that is smaller/larger. 
      c:\@Work\Perl\monks>perl -wMstrict -le
"my @array = (-9e9-123, -9e9-234);
 ;;
 my ($largest, $smallest) = ($array[0], $array[0]);
 ;;
 for my $elem (@array) {
   $largest  = $elem if $elem > $largest;
   $smallest = $elem if $elem < $smallest;
   }
 print qq{smallest: $smallest;  largest: $largest};
"
smallest: -9000000234;  largest: -9000000123

      [download]

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

[reply]
[d/l]
[select]

        You're right. I should have pointed the dangers of my assumptions in my post. I hadn't even thought of just using the 0th element of the array twice to initialize the largest/smallest -- but that's the best choice for the single-loop version. Thanks.

        TIMTOWTDI: with perl 5.24 and newer, you could set those to my ($largest, $smallest) = (-POSIX::Inf, +POSIX::Inf): nothing's bigger or smaller than Infinity, after all. :-)

[reply]
[d/l]
Re: resetting a foreach loop!
by Anonymous Monk on Nov 17, 2017 at 13:32 UTC
    Finding the largest and smallest values in an array can be done using a single sequential pass through it, without sorting.
[reply]

Back to Seekers of Perl Wisdom