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

I have a question, and I think the answer will clear up a lot aobut subroutines for me. I am writing a program that asks for a simple addition problem, stores the integers for the addition problem in variables, converts them to words, and displays the problem and the answer in word form. I call the same subroutine for the second integer as I do for the sum, but the subroutine only works for first integer. It doesn't return anything for the sum. Can anybody tell me why?
#!/usr/bin/perl -w use strict ; my (@a, $userin, $userin2, $sum) ; @a = (1 .. 9) ; print "This program will ask you to type in two numbers and then add t +hose numbers together.\nIt will then display the problem and the answ +er in word form.\n" ; $userin = 0 ; while ($userin ne "done") { print "Type in a single digit number between one and four, or type + done to exit.\n" ; chomp ($userin = <STDIN>) ; if ($userin eq "done") { last ; } print "Type in a single digit number between one and five\n" ; chomp ($userin2 = <STDIN>) ; if (($userin =~ /^[^1-4]$/) || ($userin2 =~ /^[^1-5]$/)) { print "I do not understand\n" ; } else { $sum = ($userin + $userin2) ; $userin = (firstnumtoword($userin)) ; $userin2 = (numtoword($userin2)) ; $sum = (numtoword($sum)) ; print "$userin plus $userin2 equals $sum\n" ; } } sub numtoword { $_ = shift ; my (%nums) ; %nums = ("1"=>"one", "2"=>"two", "3"=>"three", "4"=>"four", "5"=>" +five", "6"=>"six", "7"=>"seven", "8"=>"eight", "9"=>"nine") ; return $nums{$userin2} ; } sub firstnumtoword { $_ = shift ; my (%nums2) ; %nums2 = ("1"=>"One", "2"=>"Two", "3"=>"Three", "4"=>"Four", "5"=> +"Five", "6"=>"Six", "7"=>"Seven", "8"=>"Eight", "9"=>"Nine") ; return $nums2{$userin2} ; }

Replies are listed 'Best First'.
Re: Subroutine Question
by GrandFather (Saint) on Jun 27, 2012 at 05:02 UTC

    The digit to word conversion is a trivial hash lookup so that doesn't need a sub, but the input prompt and checking is complicated so that is worth writing a sub for. Consider:

    use strict; use warnings; my %toWord = ( "1" => "one", "2" => "two", "3" => "three", "4" => "four", "5" => "five", "6" => "six", "7" => "seven", "8" => "eight", "9" => "nine" ); print <<MSG; This program will ask you to type in two numbers and then add those nu +mbers together. It will then display the problem and the answer in word form. MSG while (1) { my $user1 = getDigit(4, ' or type done to exit') or next; last if $user1 eq "done"; my $user2 = getDigit(5) or next; my $sum = ($toWord{$user1 + $user2}); $user1 = (ucfirst $toWord{$user1}); print "$user1 plus $toWord{$user2} equals $sum\n"; } sub getDigit { my ($last, $extra) = @_; $extra .= ': '; print "Enter and integer between one and $toWord{$last}$extra"; chomp(my $value = <STDIN>); if ($value !~ /^([1-9]|done)$/) { print "I don't understand '$value'\n"; return; } return $value if $value =~ /^done$/ || $value <= $last; print "$toWord{$value} is not in the range one to $toWord{$last}.\ +n"; return; }

    Note that variables are declared where they are first used.

    True laziness is hard work
      I wrote it with a subroutine because I am in the chapter regarding subroutines in the textbook that I am walking myself through. I know that this program could have been accomplished in other ways.

        There are two elements to my reply. One is to provide examples of a few techniques you may not have encountered yet (like using HEREDOCs in the print statements) and using if as a statement modifier (last if ...;).

        The second element is to think about appropriate places to use a subroutine. Subs are generally used either to wrap up common code so it is only implemented once, or to wrap up some complex code so that it is easier to manage and understand. In either case using a sub makes code much easier to test because the sub can be tested in isolation.

        Part of the Perl philosophy is TIMTOWTDI (there is more than one way to do it). However some ways are better than others and, especially while you are learning, it is good to see a variety of alternative solutions. In this case the original sample duplicated code by having two essentially identical subs, but didn't take advantage of a sub to wrap up the complicated task of getting and validating data from the user. Figuring out good places to use subs is quite an art and requires a fair degree of experience. Don't be afraid to change your mind about where and how to use subs in your code. Very often you can make code much easier to understand and maintain through refactoring (moving the code around) where subs play a very large part in how code is managed.

        True laziness is hard work
Re: Subroutine Question
by Anonymous Monk on Jun 27, 2012 at 03:18 UTC
Re: Subroutine Question
by muba (Priest) on Jun 27, 2012 at 04:50 UTC

    The problem exists because numtoword draws from $userin2 both times, instead of getting information from the arguments passed to the subroutine call.

    I've taken the liberty to rewrite your program. During which, a question came up. What's the purpose of @a in your original code? I don't see it being used anywhere again. You declare it with my (@a, ...);, you define it with @a = (1 .. 9);, and then it just sits there doing nothing.

    I ran my version of the progam, and deliberately gave some invalid answers a couple of times, just to show that those are neatly caught and handled.

    Type in a single number between one and four >one Enter 1, 2, 3, or 4 please. Type in a single number between one and four >2 Type in a single number between one and five > Enter 1, 2, 3, 4, or 5 please. Type in a single number between one and five >55 Enter 1, 2, 3, 4, or 5 please. Type in a single number between one and five >5 Two plus five equals seven.
Re: Subroutine Question
by frozenwithjoy (Priest) on Jun 27, 2012 at 03:17 UTC
    You need to change return $nums{$userin2}; to return $nums{$_}; in both subroutines (but you only need one sub).
      I just made the correction recommended by frozenwithjoy and now my program is returning " plus equals " I am more confused than ever. Before, my return variable shared the same name as the variable that I inputed into the subroutine with shift. Then, I noticed that the variable name associated with shift seemed to be irelevant so I changed it to $_ and everything worked except for sum. Just now, for congruency's sake, I changed the return variable to $_ and now nothing works. By what logic is this hapening? This is a fascinating bug to me.

        Hmmm. It works great for me. Are you certain that you made the correct changes and no others? Before my changes, the output is:

        This program will ask you to type in two numbers and then add those nu +mbers together. It will then display the problem and the answer in word form. Type in a single digit number between one and four, or type done to ex +it. 2 Type in a single digit number between one and five 4 Use of uninitialized value $sum in concatenation (.) or string at ./su +m2.pl line 23, <STDIN> line 2. Four plus four equals Type in a single digit number between one and four, or type done to ex +it. done

        Notice how I input 2 and 4, but it only saw 4 and 4. That is because you were using </c>$userin2</c> in your return. After the changes, the output is:

        This program will ask you to type in two numbers and then add those nu +mbers together. It will then display the problem and the answer in word form. Type in a single digit number between one and four, or type done to ex +it. 2 Type in a single digit number between one and five 4 Two plus four equals six Type in a single digit number between one and four, or type done to ex +it. done

        Here is the altered script. The only changes are on lines 20 and 32 (also 2nd subroutine is commented out).

        #!/usr/bin/perl -w use strict ; my (@a, $userin, $userin2, $sum) ; @a = (1 .. 9) ; print "This program will ask you to type in two numbers and then add t +hose numbers together.\nIt will then display the problem and the answ +er in word form.\n" ; $userin = 0 ; while ($userin ne "done") { print "Type in a single digit number between one and four, or type + done to exit.\n" ; chomp ($userin = <STDIN>) ; if ($userin eq "done") { last ; } print "Type in a single digit number between one and five\n" ; chomp ($userin2 = <STDIN>) ; if (($userin =~ /^[^1-4]$/) || ($userin2 =~ /^[^1-5]$/)) { print "I do not understand\n" ; } else { $sum = ($userin + $userin2) ; $userin = ucfirst (numtoword($userin)) ; #was: $userin = (f +irstnumtoword($userin)) ; $userin2 = (numtoword($userin2)) ; $sum = (numtoword($sum)) ; print "$userin plus $userin2 equals $sum\n" ; } } sub numtoword { $_ = shift ; my (%nums) ; %nums = ("1"=>"one", "2"=>"two", "3"=>"three", "4"=>"four", "5"=>" +five", "6"=>"six", "7"=>"seven", "8"=>"eight", "9"=>"nine") ; return $nums{$_} ; #was: return $nums{$userin2} ; } # sub firstnumtoword { # $_ = shift ; # my (%nums2) ; # %nums2 = ("1"=>"One", "2"=>"Two", "3"=>"Three", "4"=>"Four", "5" +=>"Five", "6"=>"Six", "7"=>"Seven", "8"=>"Eight", "9"=>"Nine") ; # return $nums2{$userin2} ; # }

        Also, it would be more readable to write your subroutine like this:

        sub numtoword { my $digit = shift; my %nums; %nums = ( "1"=>"one", "2"=>"two", "3"=>"three", "4"=>"four", "5"=> +"five", "6"=>"six", "7"=>"seven", "8"=>"eight", "9"=>"nine" ); return $nums{$digit};
        Could it be that perhaps $sum was bigger than 9? If so, there is nothing in the hash to find and you get an undef value returned.

        Have a look at Lingua::Any::Numbers. It can convert numbers to strings.

        CountZero

        A program should be light and agile, its subroutines connected like a string of pearls. The spirit and intent of the program should be retained throughout. There should be neither too little or too much, neither needless loops nor useless variables, neither lack of structure nor overwhelming rigidity." - The Tao of Programming, 4.1 - Geoffrey James

        My blog: Imperial Deltronics

        i tried what frozenwithjoy has suggested to you and it worked superbly well for me. can you post the code you have made to the changes to...

        *=*=*dEEPAk*=*=*