Return value from nested Subroutine Perl

Question

We've received an assignment from school, where we're supposed to make our own small n' simple Perl application. I thought I'd make a ATM simulator. So far, it's been going great; I've created a menu (Withdraw, Balance, Transfer), by using subroutines. This is my code so far:

#! /usr/bin/perl
#Written by: Tobias Svenblad, h15tobsv@du.se, Digitalbrott & e-Säkerhetsprogrammet (2015)
#PerlLab03-2c.plx

use warnings;
use strict;
use Term::ANSIColor;
use Text::Format;

my $firstname;
my $lastname;
my $acc_balance = 2451.26;
my $acc_withdraw;
my $clr_scr = join( "", ( "\033[2J", "\033[0;0H" ) );    #This variable will clear the screen and jump to postion 0, 0.

my $atm = Text::Format->new;
print color('green');
print $atm->center("ATM v. 1.20");
print color('reset');

#Create account message.
my $crt_acc_msg = <<"END_MSG";
\nDear Sir or Madam,\n
We're very happy you've chose us as your bank.
Before we proceed, we need to set-up your account.\n
END_MSG

print $crt_acc_msg;

&acc_create;
&acc_choose;

sub acc_create {
  ACC_BEGINNING:

    #First name:
    print "\nYour first name: ";
    $firstname = <STDIN>;
    chomp $firstname;

    #Last name:
    print "\nYour last name: ";
    $lastname = <STDIN>;
    chomp $lastname;
    if ( defined($firstname) && $firstname ne "" ) {
        if ( defined($lastname) && $lastname ne "" ) {
            goto ACC_PASS;
        }
    }
    else {
        print "You didn't fill in first or last name. Try again. \n";
        goto ACC_BEGINNING;
    }
  ACC_PASS:
    print "Please wait while the system loads.\n\n";

    #sleep(2);
    print $clr_scr;

    print color('green');
    print $atm->center("ATM v. 1.20");
    print color('reset');

    print "\nWelcome ", $firstname, " ", $lastname, "!\n\n";
}

sub acc_choose {

    sub acc_balance {
        print $clr_scr;

        print color('green');
        print $atm->center("ATM v. 1.20");
        print color('reset');

        print "\nYour balance is: ";
        print color('green');
        print $acc_balance;
        print color('reset');
        print " SEK\n\n";
        &acc_choose;
    }

    sub acc_withdraw {

      ENTER_AMOUNT:
        print $clr_scr;

        print color('green');
        print $atm->center("ATM v. 1.20");
        print color('reset');

        print "\nEnter how much you'd like to withdraw: \n";
        my $acc_balance_withdraw = <STDIN>;

        if ( $acc_balance_withdraw > $acc_balance ) {
            print "Insufficient funds.";
            goto ENTER_AMOUNT;
        }
        $acc_balance -= $acc_balance_withdraw;
        print "\nYour current balance is now: ";
        print color('green');
        print $acc_balance;
        print color('reset');
        print " SEK\n\n";
        &acc_choose;
    }

    sub acc_transfer {
      ENTER_AMOUNT:
        print $clr_scr;

        print color('green');
        print $atm->center("ATM v. 1.20");
        print color('reset');

        print "\nEnter how much you'd like to transfer: \n";
        my $acc_balance_withdraw = <STDIN>;

        if ( $acc_balance_withdraw > $acc_balance ) {
            print "Insufficient funds.";
            goto ENTER_AMOUNT;
        }

        print "\nYour current balance is now: ";
        print color('green');
        print $acc_balance - $acc_balance_withdraw;
        print color('reset');
        print " SEK\n\n";
        &acc_choose;
    }
  ACC_CHOOSE:
    print "[ ";
    print color('cyan');
    print "1";
    print color('reset');
    print " ]";
    print "Account Balance\n";
    print "[ ";
    print color('cyan');
    print "2";
    print color('reset');
    print " ]";
    print "Withdraw\n";
    print "[ ";
    print color('cyan');
    print "3";
    print color('reset');
    print " ]";
    print "Transfer\n";
    my $choice1 = <STDIN>;
    chomp $choice1;

    if ( $choice1 == 1 ) {
        &acc_balance;
    }
    elsif ( $choice1 == 2 ) {
        &acc_withdraw;
    }
    elsif ( $choice1 == 3 ) {
        &acc_transfer;
    }
    else {
        print "You entered an invalid option. Try again. \n";
        goto ACC_CHOOSE;
    }
    return ();
}

The problem I face is when I try to return the $acc_balance value to the other subroutines. I've tried to implement return($acc_balance ); underneath the nested subroutines, but that'll just prompt me to end the application. So basically, what I'm trying to do is to update $acc_balance every time I make a withdrawal or transfer (they are both currently the same thing in this code), but whenever I try to do that, it either doesn't update the value or it'll just show the classic "Press any key to continue..." message.

Any help is greatly appreciated! Thanks!

Answer 1

I think you shouldn't be using subroutines for this assignment. But it worries me that someone has told you to use an ampersand & when you call a subroutine and has explained how to use labels and goto. That's inappropriate for a modern computer language and you can do much better

For instance, here's how I would write your subroutine acc_create

sub acc_create {

    while () {

        print "\nYour first name: ";
        chomp (my $firstname = <STDIN>);

        print "\nYour last name: ";
        chomp (my $lastname = <STDIN>);

        last if $firstname and $lastname;

        print "You didn't fill in first or last name. Try again.\n";
    }

    print "Please wait while the system loads.\n\n";

    print
        $clr_scr,
        color('green'), $atm->center("ATM v. 1.20"), color('reset');

    print "\nWelcome $firstname $lastname!\n\n";
}

There's a lot more I could say, but Stack Overflow isn't the place for a tutorial

Answer 2

I removed the formatting and colouring, as it's irrelevant to the question. Here's how you can do it: everything a subroutine needs gets from its arguments, everything it changes is returned.

#! /usr/bin/perl
use warnings;
use strict;
use feature qw{ say };

sub create {
    print << 'END_MSG';

Dear Sir or Madam,

We're very happy you've chose us as your bank.
Before we proceed, we need to set-up your account.

END_MSG

    my ($firstname, $lastname);

    my $first = 1;
    while (! defined $firstname || ! defined $lastname
           || q() eq $firstname || q() eq $lastname
          ) {
        say "You didn't fill in first or last name. Try again." unless $first;
        undef $first;

        print "\nYour first name: ";
        chomp( $firstname = <STDIN> );

        print "\nYour last name: ";
        chomp( $lastname = <STDIN> );
    }

    say "Please wait while the system loads.\n";

    say "\nWelcome ", $firstname, " ", $lastname, "!\n";
    return ($firstname, $lastname)
}

sub choose {
    my $balance = 2451.26;
    my @menu = ( 'Account Balance',
                 'Withdraw',
                 'Transfer',
                 'Quit',
               );
    my $choice = q();

    until ('4' eq $choice) {
        for my $i (0 .. $#menu) {
            say '[ ', $i + 1, ' ] ', $menu[$i];
        }

        chomp( $choice = <STDIN> );

        my @actions = (\&balance,
                       \&withdraw,
                       \&transfer,
                       sub {}
                      );
        my $action = $actions[$choice - 1];

        if ($action) {
            my $value = $action->($balance);
            $balance = $value if defined $value;
        } else {
            say 'You entered an invalid option. Try again.';
        }
    }
}

sub balance {
    my $balance = shift;
    say "\nYour balance is: $balance SEK\n\n";
    return $balance
}

sub withdraw {
    my $balance = remove('withdraw', @_);
    return $balance
}

sub transfer {
    my $balance = remove('transfer', @_);
    return $balance
}

sub remove {
    my ($action, $balance) = @_;

    say "\nEnter how much you'd like to $action:\n";
    my $remove = <STDIN>;

    if ( $remove > $balance ) {
        print "Insufficient funds.";
    } else {
        $balance -= $remove;
    }
    balance($balance);
}

my ($firstname, $lastname) = create();
choose();
say "Good bye, $firstname $lastname!";

Nested subroutines are not nested the same way as in Pascal. In fact, you can't nest named subroutines in Perl.

As shown, only use & when you need a reference to a subroutine.

Don't use goto. Loops are easier to understand and manage.

Also, make sure you don't erase the screen immediately after printing an important message ("Insufficient funds").

Answer 3

Looks cute, but there are some things you shouldn't do. I'll try to show you with the choose part.

Don't call subroutines with &, but with parentheses, i.e. acc_choose(); instead of &acc_choose;.

Don't nest functions/subs in perl. If you really want to encapsulate things (which I appreciate and recommend), use modules. But that's beyond the scope of this question. You'll learn about that later.

Don't use goto if not absolutely neccessary. It makes the control flow weird, hard to follow, and often leads to surprises.

If you want to repeat something until a certain condition is met -- or, in other words -- while a certain condition is not met, use while loops.

Given that, I'd suggest something like this for the choose part (fancy printing omitted):

sub acc_balance {
    print "\nYour balance is: $acc_balance SEK\n\n";
}

sub acc_withdraw {
    my $acc_balance_withdraw = 0;

    do {
        print "\nEnter how much you'd like to withdraw: \n";
        $acc_balance_withdraw = <STDIN>;

        if ( $acc_balance_withdraw > $acc_balance ) {
            print "Insufficient funds.\n";
        }
    } while( $acc_balance_withdraw > $acc_balance );

    # if you get here, then $acc_balance_withdraw <= $acc_balance, so:      
    $acc_balance -= $acc_balance_withdraw;
    print "\nYour current balance is now: $acc_balance SEK\n\n";
}


# actually almost the same as acc_withdraw() only with
# other screen output and no `-=` operation 
sub acc_transfer {
    # left as an excercise
}


sub acc_choose {

    print "[1] Account Balance\n";
    print "[2] Withdraw\n";
    print "[3] Transfer\n";
    print "[4] Exit\n";

    do {
        my $choice1 = <STDIN>;
        chomp $choice1;

        if ( $choice1 == 1 ) {
            acc_balance();
        }
        elsif ( $choice1 == 2 ) {
            acc_withdraw();
        }
        elsif ( $choice1 == 3 ) {
            acc_transfer();
        }
        elsif ( $choice1 == 4 ) {
            print "Thank you. Good bye.\n"
        }
        else {
            print "You entered an invalid option. Try again. \n";
        }
    } while( $choice != 1 && $choice != 2 && $choice != 3 && $choice != 4 );
}

One issue with your attempt might be that you called acc_choose() recursively, i.e. you called it from within itself. Together with goto that can indeed have entertaining effects.

Good luck and keep on.

Oh, and to answer your question: It is now really simple to have the subroutines return something. It is not neccessary here because you use global variables for $acc_balance (don't do that either), but if you'd like to, you could let the subs return the new balance like so:

sub acc_withdraw {
    my $old_balance = shift; # that's the first argument given to this sub
    my $acc_balance_withdraw = 0;

    do {
        print "\nEnter how much you'd like to withdraw: \n";
        $acc_balance_withdraw = <STDIN>;

        if ( $acc_balance_withdraw > $old_balance ) {
            print "Insufficient funds.\n";
        }
    } while( $acc_balance_withdraw > $old_balance );

    # if you get here, then $acc_balance_withdraw <= $acc_balance, so:      
    my $new_balance = $old_balance - $acc_balance_withdraw;
    print "\nYour current balance is now: $new_balance SEK\n\n";
    return $new_balance;
}

# and then...

$acc_balance = acc_withdraw($acc_balance);