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

I've written the following working script for counting money. You tell it how many coins and notes you have and it calculates how much money you have. The problem is I'm sure the script is much longer than it needs to be and I'm unsure how to make it shorter, any help would be greatly appreciated. I've only been working with perl for a few weeks so my knowledge isn't too good at the moment. Thanks in advance.
#!usr/bin/perl @pence=qw(5 10 20 50); @notels=qw(5 10 20 50); $note = $notes[0]; $coin = $pence[0]; sub pence{ print "How many $coin pence pieces do you have? "; $num=<STDIN>; chomp $num; $res = ($coin * $num)/100; } sub notes{ print "How many £$note notes do you have? "; $num=<STDIN>; chomp $num; $res = ($note * $num); } $coin = $pence[0]; &pence; $five=$res; $coin = $pence[1]; &pence; $ten=$res; $coin = $pence[2]; &pence; $twenty=$res; $coin = $pence[3]; &pence; $fifty=$res; $coin = 1; print "How many £1 coins do you have? "; $num=<STDIN>; chomp $num; $res = ($coin * $num); $coin = 2; print "How many £2 coins do you have? "; $num=<STDIN>; chomp $num; $res = ($coin * $num); $twopound = $res; $note=$notels[0]; &notes; $fivenote=$res; $note=$notels[1]; &notes; $tennote=$res; $note=$notels[2]; &notes; $twentynote=$res; $note=$notels[3]; &notes; $fiftynote=$res; $total = ($five + $ten + $twenty + $fifty +$pound + $twopound + $fiven +ote + $tennote + $twentynote + $fiftynote); print "You have £$total.\n";

Replies are listed 'Best First'.
Re: Can someone help me make this script more efficient
by ikegami (Patriarch) on Apr 13, 2008 at 17:26 UTC

    Some quick comments:

    • You should look at using return rather than using a global variable to return results from a function.

    • Just like you need to learn how to return values to a function, you need to learn how to pass arguments to a function (such as $coin to pence).

    • Your teacher or teaching material is giving you some bad advice if it advocates using & on function calls. The & is not just decorative.

    • use strict; will reveal many errors, easily fixable by using my. You should be using use warnings; as well.

Re: Can someone help me make this script more efficient
by jasonk (Parson) on Apr 13, 2008 at 17:43 UTC

    Hmm, do you just want it shorter and still produce the wrong total, or shorter and produce the correct total as well?

    # Let's see how much money I would have if I just won the # lottery and found a 2 pound coin in the couch cushions... How many 5 pence pieces do you have? 0 How many 10 pence pieces do you have? 0 How many 20 pence pieces do you have? 0 How many 50 pence pieces do you have? 0 How many £1 coins do you have? 1000000000 How many £2 coins do you have? 1 How many £5 notes do you have? 0 How many £10 notes do you have? 0 How many £20 notes do you have? 0 How many £50 notes do you have? 0 You have £2. # Hmm, guess I won't spend my £2 on a lottery ticket then...

    strict would have helped you out there... In any case...

    #!/usr/bin/perl -w use strict; use warnings; use List::Util qw( sum ); printf( "You have £%.02f\n", sum ( ( map { ask( "$_ pence pieces", $_/100 ) } ( 5, 10, 20, 50 ) ), ( map { ask( "£$_ coins", $_ ) } ( 1, 2 ) ), ( map { ask( "£$_ notes", $_ ) } ( 5, 10, 20, 50 ) ), ) ); sub ask { print "How many $_[0] do you have? " ; <STDIN> * $_[1] }

    www.jasonkohles.com
    We're not surrounded, we're in a target-rich environment!
Re: Can someone help me make this script more efficient
by moritz (Cardinal) on Apr 13, 2008 at 17:17 UTC
    You have many variables that are used in very similar ways. Consider using an array instead.
Re: Can someone help me make this script more efficient
by Corion (Patriarch) on Apr 13, 2008 at 17:13 UTC

    This smells very much like homework. Most likely, ways of shortening a script have been covered in your course, but I let you in on the number one secret to shortening a script:

    To shorten a script, you remove lines from it.

    Most likely, you want to keep the current functionality of the script, so maybe you want to not only remove lines from it but rework other lines so you can reuse them. For example, the handling of pence, quid notes, and coins can likely be folded into one routine that takes as parameters the monetary units and the factor to multiply with. Maybe you want to store all the different money as one single number too.

Re: Can someone help me make this script more efficient
by FunkyMonk (Bishop) on Apr 13, 2008 at 17:26 UTC
    In Perl, a hash is structure that contains a list of pairs of related things. For example, you could have a hash that had a list of coins/notes and their value, something like:
    5 => "5p coin", 500 => "£5 note",

    I'm sure using such a thing would make your script more efficicent.

    See Variable names for a discussion on hashes, and Perl Data Structures Cookbook for more details and examples of hashes.

Re: Can someone help me make this script more efficient
by oko1 (Deacon) on Apr 13, 2008 at 20:19 UTC

    I shouldn't. I really shouldn't. ... Oh, what the heck. :)

    #!/usr/bin/perl -w use strict; die "Usage: ", $0 =~ /([^\/]+)$/, " <[1|2|5|10|20|50]l | [5|10|20|50]p +> [...]\n" unless @ARGV; my $sum; for (@ARGV){ die "$_ is an invalid amount/type of currency!\n" unless /^(5|[125]0(?=p)|[125]0?(?=l))([pl])$/i; $sum += $1 / ${{ p => 100, l => 1 }}{lc $2}; } print "The total is L$sum.\n";
    
    ben@Tyr:/tmp$ ./pocketful_of_rye 20l 5p 20P 50p 50L 10l 5l
    The total is L85.75.
    

    The above is just a bit geek-snarky, but the point I would like to get across is that you'll want to rethink the structure of your program before you even start thinking about efficiency. Perl will execute it efficiently enough; you're not running millions of calculations per second anyway, or anything else where efficiency would matter. Even if this script took a full second to run, you wouldn't miss it. Program design, however, is critically important - not that it's specific to Perl in any way, but it's a critical aspect of any sort of programming that you'll ever do.

    Even if your script is just homework - which I strongly suspect it is - there's no reason to not give it your best shot: this is where and how you learn to do the real thing. Before you ever put your hands to the keyboard, think - I mean really think - about the task at hand. Conciseness is good; so is clarity (one of the bits that makes the above program snarky is that I blithely stomped all over clarity in the interest of conciseness. :) Thinking about the operations you have to perform on the data and combining the common ones is also a reasonable approach (again, illustrated somewhat by the above despite the bit of Perl golf with the anonymous hash.)

    Consider the user interface, too. Does it make sense to interrogate the user about every single possibility, as you do - or would it work better to let them specify the data in one shot, if at all possible? It seems to me that the range of what you're asking for is simple enough - the regular expression that I used covers everything you did in your script (the snarkiness there consists of requiring you, if you want to understand it, to run perldoc perlre and read it carefully.) This also allows you to validate the user data - always a good practice while coding.

    I could go on for a bit, but I think you get the point. Think about the task; split it into chunks that are easily comprehensible (both mentally and in code); and write the code to handle those chunks. After ten or twenty years of doing this, start focusing on efficiency - if you're still concerned with it, that is.

    Meanwhile, keep reading Perlmonks. There are lots of people here, many of them with good ideas and good coding habits. Stick around and learn - and remember to label your homework as such. It'll get you more help and less snarkiness. :)

    
    -- 
    Human history becomes more and more a race between education and catastrophe. -- HG Wells
    
Re: Can someone help me make this script more efficient
by Anonymous Monk on Apr 14, 2008 at 07:14 UTC
    or you can do it something like this
    use strict; use warnings; my @pence=qw(5 10 20 50); my @notes=qw(5 10 20 50); my ($sum1, $sum2); foreach my $c (@pence){ print "How many $c pence pieces do you have? "; my $num=<STDIN>; chomp $num; my $res = ($c * $num)/100; $sum1 += $res; } foreach my $n (@notes){ print "How many &#321;$n notes do you have? "; my $num=<STDIN>; chomp $num; my $res = ($n * $num); $sum2 += $res; } my $final =$sum1+$sum2; print "You are ritch, you have $final coins!!!!!";
    it is a bit longer but intuitive.