Beefy Boxes and Bandwidth Generously Provided by pair Networks
"be consistent"

Any improvement over the code possible??

by Rohit Jain (Sexton)
on Sep 21, 2012 at 16:33 UTC ( #994941=perlquestion: print w/replies, xml ) Need Help??
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 $_, ", "; # }
Is there any room for improvement in this code in some way??

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:

      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');

      I haven't studied till packages and modules.. So, don't know exactly how to use them..

        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.

        Super Search is another way to find answers and see code examples.

Re: Any improvement over the code possible??
by davido (Archbishop) 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; }

    Update: s/&&/and/ to fix a precedence issue.


      my @numbers = (); say average(@numbers);
      perl -E'sub Monkey::do{say$_,for@_,do{($monkey=[caller(0)]->[3])=~s{::}{ }and$monkey}}"Monkey say"->Monkey::do'
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, @_; }

    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 / @_; }

    Check for an empty list at the top to avoid a division by zero error.

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

Log In?

What's my password?
Create A New User
Node Status?
node history
Node Type: perlquestion [id://994941]
Approved by toolic
and all is quiet...

How do I use this? | Other CB clients
Other Users?
Others perusing the Monastery: (10)
As of 2017-05-25 21:05 GMT
Find Nodes?
    Voting Booth?