Any improvement over the code possible??

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

I have a problem: To read a list of numbers from user, and print all the above average numbers..

Here's is the code I have come up with.. And its working fine:

#!/perl/bin
use v5.14;

sub find_above_average {
    my $mean = &calculate_average(@_);
    say "Average is: $mean";
    
    my @numList;
    foreach (@_) {
        if ($_ > $mean) {
            push(@numList, $_);
        }
    }
    return @numList;
}

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

say "Enter a list of numbers to calculate above average numbers:";
chomp(my @numList = <STDIN>);

say "The numbers above average are: ";
print &find_above_average(@numList);

# foreach (&find_above_average(@numList)) {
    # print $_, ", ";
# }
[download]

Is there any room for improvement in this code in some way??

Comment on Any improvement over the code possible?? Download Code

Replies are listed 'Best First'.
Re: Any improvement over the code possible?? by toolic (Bishop) on Sep 21, 2012 at 16:50 UTC
Add input checking. If I input "2" and "F", the code tells me the average is 1. That is probably not how you want the code to behave. Look at Scalar::Util::looks_like_number. See also: perlcritic	[reply]
Re^2: Any improvement over the code possible?? by Rohit Jain (Sexton) on Sep 21, 2012 at 17:08 UTC
Thanks for reply :) I tried out this code.. but its not giving what I expected. Is it how it works?? `#!/perl/bin use v5.14; use Scalar::Util; say &Scalar::Util::looks_like_number('2');` [download] I haven't studied till packages and modules.. So, don't know exactly how to use them..	[reply] [d/l]
Re^3: Any improvement over the code possible?? by toolic (Bishop) on Sep 21, 2012 at 17:18 UTC
its not giving what I expected You need to tell us what you expect it to do. Or read the doc I linked to: `looks_like_number EXPR Returns true if perl thinks EXPR is a number.` [download] Super Search is another way to find answers and see code examples.	[reply] [d/l]
Re^4: Any improvement over the code possible?? by Rohit Jain (Sexton) on Sep 21, 2012 at 17:23 UTC
Re^5: Any improvement over the code possible?? by Rohit Jain (Sexton) on Sep 21, 2012 at 17:42 UTC
Some notes below your chosen depth have not been shown here
Re: Any improvement over the code possible?? by davido (Cardinal) on Sep 21, 2012 at 18:08 UTC
I think I'd like to see a looser coupling between find_above_average and calculate_average. And I would move the prints outside of the subroutines, to make them more generic. use v5.14; use warnings; use IO::Interactive qw( is_interactive ); use Scalar::Util qw( looks_like_number ); use List::Util qw( sum ); say "Enter a list of numbers to find above average numbers, one per li +ne:" if is_interactive; my @numbers = get_input(); say "No numbers entered. Exiting" and exit if ! @numbers; my $average = average( @numbers ); say "The average is $average"; say "The above-average numbers are:"; say "\t$_" for above_average( $average, @numbers ); sub average { return sum( @_ ) / @_; } sub above_average { my $average = shift; return grep { $_ > $average } @_; } sub get_input { chomp ( my @numlist = <STDIN> ); return grep { length $_ && looks_like_number($_) } @numlist; } [download] Update: `s/&&/and/` to fix a precedence issue. Dave	[reply] [d/l] [select]
Re^2: Any improvement over the code possible?? by tobyink (Canon) on Sep 21, 2012 at 20:18 UTC
`my @numbers = (); say average(@numbers);` [download] `perl -E'sub Monkey::do{say$_,for@_,do{($monkey=[caller(0)]->[3])=~s{::}{ }and$monkey}}"Monkey say"->Monkey::do'`	[reply] [d/l]
Re: Any improvement over the code possible?? by Kenosis (Priest) on Sep 21, 2012 at 18:00 UTC
Consider the following: `sub find_above_average { my $mean = &calculate_average(@_); return grep $_ > $mean, @_; }` [download] Remove the `say` line and print the average outside of the subroutine, and use `grep` to `return` only those values greater than the value of `$mean`. `sub calculate_average { @_ > 0 or die 'No numbers sent for averaging.'; my $sum = 0; foreach (@_) { $sum += $_; } return $sum / @_; }` [download] Check for an empty list at the top to avoid a division by zero error.	[reply] [d/l] [select]
Re: Any improvement over the code possible?? by bulk88 (Priest) on Sep 21, 2012 at 20:51 UTC
Use array references to avoid pass/return by copy???	[reply]