http://www.perlmonks.org?node_id=126819


in reply to Seeking Feed Back on a Game

Well, let's see wht we have. It's a good idea to take any repeated code and try making a sub out of it, so let's start there.

One of the first things we notice is:
print "What Goal In Cash Do You Want To Set? :"; chomp ($goal=<STDIN>);
Let's make this a bit easier to use with a sub.
sub query_user { print @_ if @_; chomp (my $answer = <STDIN> ); return $answer; }
Now we can use:
my $goal = query_user( qq|\nWhat Goal In Cash Do You Want To Set? :| );

This reminds me, why not create a notify_user subroutine also. That way if we wish to convert this game to TK or the dreaded windows, we would only have to change the notify_user subroutine and leave the rest of the code alone.

sub notify_user { return print @_; }
And then we need to change query_user to:
sub query_user { notify_user(@_) if @_; chomp (my $answer = <STDIN> ); return $answer; }

Now that we have isolated the output, let's take a look at something a bit more complicated. All those if blocks are shouting data structure. If our data structure were changed we might condense those blocks to a single if block within, perhaps, a loop.

Each programmer will have a different structure. We'll use mine because it's the only one my ego will allow. Since this is a stock simulation, I decided to use a hash called %portfolio to represent the players holdings. I decided to use %ticker to represent the price and volitility of each stock. Last, I used @stock_names as a list of stock names.

This is how I chose to fill them:

my (%ticker, %portfolio); my @stock_names = ('Derpco', 'Tetrabytz', 'Mojo Inc', 'Freke', 'Mega') +; @portfolio{@stock_names} = (0) x @stock_names; @ticker{@stock_names} = map { price => generate_random(10, 100), volitility => int(rand(7)) * 5 + 5 }, @stock_names;
This replaces the following code:
@stocks = (0,0,0,0,0); $derpco=genRand(10,100); $tetrabytz=genRand(10,100); $mojoinc=genRand(10,100); $freke=genRand(10,100); $mega=genRand(10,100); $cash=genRand(500,1000);

And it randomizes the volotility of the stocks in @stock_names. No doubt you've noticed the name change in generate_random. Each programmer follows his or her own set of style rules. A sample style is in perlsyn. These rules might include indentation, placement of subroutines, documentation and other rules to make programs easier to read and be worked on by a programming team.

One style rule I like is to not use mixed case variables and subroutines. Sometimes this is difficult, like the Tr function in CGI.pm. Usually I stick to lowercase letters and I separate words with _ (the underscore). If someone who speaks a different language might look at my script, I'll usually spell the whole word: (as opposed to gen_rand). It seems to more common today for foriegn language speakers to review scripts written in english.

Let's see what we have so far. This is what we started with:

#!/usr/bin/perl -w use strict; my($dif,$randNumber,$manip,$whichstock,$howmany,$goal,$date,$derpco,$t +etrabytz,$mojoinc,$freke,$mega,$cash,@stocks,$buy,$sell,$buynum,$sell +num,$trans); @stocks = (0,0,0,0,0); sub genRand { my $startNumber = shift(); my $endNumber = shift(); my $randNumber = int($startNumber + rand() * ($endNumber - $startNumbe +r)); return $randNumber; }; print "What Goal In Cash Do You Want To Set? :"; chomp ($goal=<STDIN>); print "Thanks\n\n"; $date=1; $derpco=genRand(10,100); $tetrabytz=genRand(10,100); $mojoinc=genRand(10,100); $freke=genRand(10,100); $mega=genRand(10,100); $cash=genRand(500,1000);
And this is what we have now:
#!/usr/bin/perl use strict; use diagnostics; my (%ticker, %portfolio); my @stock_names = ('Derpco', 'Tetrabytz', 'Mojo inc', 'Freke', 'Mega') +; @portfolio{@stock_names} = (0) x @stock_names; @ticker{@stock_names} = map { price => generate_random(10, 100), volitility => int(rand(7)) * 5 + 5 }, @stock_names; my $goal = query_user( qq|\nWhat Goal In Cash Do You Want To Set? :| ) +; notify_user( qq|Thanks\n\n| ); my $date = 0; my $cash = generate_random(500, 1000); sub generate_random { my ($start, $end) = @_; return int( $start + rand($end - $start + 1) ); } sub query_user { notify_user(@_) if @_; chomp (my $answer = <STDIN> ); return $answer; } sub notify_user { return print @_; }
Note: use diagnostics; will turn on warnings.

generate_random would be easier to use if it accepted a single arguement for voltility calculations. For instance, generate_random(10) might return the same as generate_random(-10, 10). To allow for a single argument we could test if $end is defined.

unless ( defined $end ) { $end = $start; $start = 0 - $end; }
which can be reduced to:
$end = 0 - ($start = 0 - $start) unless defined $end; Really!
sub generate_random { my ($start, $end) = @_; $end = 0 - ($start = 0 - $start) unless defined $end; return int( $start + rand($end - $start + 1) ); }
This will allow us to replace:
$manip=genRand(0,50); $derpco=$derpco+genRand(-10,10); $tetrabytz=$tetrabytz+genRand(-40,40); $mojoinc=$mojoinc+genRand(-20,20); $freke=$freke+genRand(-25,25); $mega=$mega+genRand(-35,35); if ($derpco < 1){$derpco = 0; $stocks[0]=0; }; if ($tetrabytz < 1){$tetrabytz = 0; $stocks[1]=0;}; if ($mojoinc < 1){$mojoinc = 0; $stocks[2]=0;}; if ($freke < 1){$freke = 0; $stocks[3]=0;}; if ($mega < 1){$mega = 0; $stocks[4]=0;};
with:
# update prices foreach my $stock ( @ticker{@stock_names} ) { $$stock{price} += generate_random($$stock{volitility}); $$stock{price} = 0 if $$stock{price} < 1; } # liquidate holdings if price hits 0 foreach my $stock ( @stock_names ) { $portfolio{$stock} = 0 if $ticker{$stock}{price} == 0 }

(I skipped the stock manipulation section of code. It didn't check to see if the manipulated stock was owned by the player and truly represented a windfall.) The first and second foreach blocks could be combined, but I left them separate to show how <NOBR>%ticker</NOBR> could be stepped through with references. If you want more information on references and data structures take a look at perlref, perlreftut, perllol, and, perldsc.

Since the stock name is variable length, I decided to change the stock report to place the names in the right column. Additionally I opted for Holdings instead of You Own for the first column heading. Here's my report generating code.

notify_user ( qq|\n\nYour Stock Report for January $date\n\n|, qq|Holdings\tValue\tCompany\n\n|); notify_user( qq|\t$portfolio{$_}\t\$$ticker{$_}{price}\t$_\n| ) for @stock_names; my $transaction = query_user( qq|\nYou have \$$cash in cash and \$|, portfolio_value( \%portfolio, \%ticker, ), qq| in stocks.\n|, qq|Would you like to (B)uy or (S)ell or are you (D)one? | );

I decided to place this in a sub call </CODE>daily_report</CODE>. The portfolio_value subroutine returns the total value of our portfolio.

sub portfolio_value { my ($portfolio, $ticker) = @_; my $total_value = 0; $total_value += $$portfolio{$_} * $$ticker{$_}{price} for keys %$portfolio; return $total_value; }

I didn't like the feature that allowed for only one transaction per day. I wanted to be able to buy and sell as much as I wanted each day, so I wrapped everything in a while (1) block.

Here's what my main block looks like:
until ( $cash > $goal or $date == 31 ) { # update prices foreach my $stock ( @ticker{@stock_names} ) { $$stock{price} += generate_random($$stock{volitility}); $$stock{price} = 0 if $$stock{price} < 1; } # liquidate holdings if price hits 0 foreach my $stock ( @stock_names ) { $portfolio{$stock} = 0 if $ticker{$stock}{price} == 0 } $date++; print qq|Last Day!! better sell your stock| if $date == 30; while (1) { notify_user(daily_report(\%portfolio, \%ticker, $cash, $date)) +; my $transaction = query_user( q|Would you like to (B)uy or (S)ell |, q|or are you (D)one? | ); if ($transaction =~/^s/i) { $cash += sell(\%portfolio, \%ticker); } elsif ($transaction =~/^b/i) { $cash = buy(\%portfolio, \%ticker, $cash); } elsif ( $transaction =~ /^d/i ) { last; } elsif ( $transaction =~ /^q/i ) { exit; } } }

Q and D will leave the while block and everything else will repeat. Let's look at the buy and sell subroutines.

sub sell { my ($portfolio, $ticker) = @_; my $stock_to_sell = query_user(q|What Stock would you like to Sell +? |); my $cash = 0; foreach my $stock ( keys %$portfolio ) { if ( $stock =~ /$stock_to_sell/i ) {; if ( $$ticker{$stock}{price} == 0 ) { notify_user( qq|$stock has filed for protection from creditors| +, q| - Your holdings were liquidated.| ); } else { my $shares_to_sell = query_user( qq|\n\nYou have $portfolio{$stock} shares in $stoc +k\n|, q|Sell How many? :| ); if ( $shares_to_sell > $$portfolio{$stock} ) { notify_user( qq|Sorry, your account is not set |, qq|up for short selling.\n| ); } else { $$portfolio{$stock} -= $shares_to_sell; $cash = $shares_to_sell * $$ticker{$stock}{price}; } } last; } } return $cash; }

If the player successfully purchases a stock we update her portfolio and return the cash generated. Our data stucture allows us to use a foreach block to eliminate all those repititious if blocks. The

buy<CODE> sub routine fixes a bug in the original game which allowed t +he player to purchase a few million shares when the price reached zer +o.</P> <CODE> sub buy { my ($portfolio, $ticker, $cash) = @_; my $stock_to_buy = query_user( q|What Stock would you like to buy? + |); foreach my $stock ( keys %$portfolio ) { if ( $stock =~/$stock_to_buy/i ) { if ( $$ticker{$stock}{price} == 0 ) { notify_user( qq|Sorry, trading has been suspended in $stock.| ) +; } else { my $shares_to_buy = query_user( qq|\n\nYou have $portfolio{$stock} shares in $stoc +k and|, qq|\$$cash\nBuy How many |, qq|(max int( $cash / $$ticker{$stock}{price} ), q| +) :| ); if ( $cash < $$ticker{$stock}{price} * $shares_to_buy +) { notify_user( qq|Sorry, this isn't a margin account +.\n| ); } else { $$portfolio{$stock} += $shares_to_buy; $cash -= $shares_to_buy * $$ticker{$stock}{price}; } } last; } } return $cash; }

I started out simply replacing the orignal code and then added enhancements like stopping the trade of stocks with no value and aadd max shares so the player doesn't have to figure it out. Subroutines make it very easy to add such features. It shouldn't be hard to add a feature to let the player to buy another or to sell all their holdings.

Finally, I chose to leave the rest alone, except to add notify_user and some indentation:
if ( $cash > $goal ) { my $dif = $cash - $goal; notify_user( qq|Yay! you won, you beat your goal by \$$dif\n| ); } if ($cash < $goal) { my $dif = $goal - $cash; notify_user( qq|Boo! you lost, you missed your goal by \$$dif\n| ) +; }
As tested:
#!/usr/bin/perl #require v5.6.1; use strict; use diagnostics; #use warnings; #use Data::Dumper; #use CGI qw/:standard/; #use CGI::Carp 'fatalsToBrowser'; my (%ticker, %portfolio); my @stock_names = ('Derpco', 'Tetrabytz', 'Mojo inc', 'Freke', 'Mega') +; @portfolio{@stock_names} = (0) x @stock_names; @ticker{@stock_names} = map { price => generate_random(10, 100), volitility => int(rand(7)) * 5 + 5 }, @stock_names; my $goal = query_user( qq|\nWhat Goal In Cash Do You Want To Set? :| ) +; notify_user( qq|Thanks\n\n| ); my $date = 0; my $cash = generate_random(500, 1000); until ( $cash > $goal or $date == 31 ) { # update prices foreach my $stock ( @ticker{@stock_names} ) { $$stock{price} += generate_random($$stock{volitility}); $$stock{price} = 0 if $$stock{price} < 1; } # liquidate holdings if price hits 0 foreach my $stock ( @stock_names ) { $portfolio{$stock} = 0 if $ticker{$stock}{price} == 0 } $date++; print qq|Last Day!! better sell your stock| if $date == 30; while (1) { notify_user(daily_report(\%portfolio, \%ticker, $cash, $date)) +; my $transaction = query_user( qq|Would you like to (B)uy or (S)ell or are yo +u (D)one? | ); if ($transaction =~/^s/i) { $cash += sell(\%portfolio, \%ticker); } elsif ($transaction =~/^b/i) { $cash = buy(\%portfolio, \%ticker, $cash); } elsif ( $transaction =~ /^d/i ) { last; } elsif ( $transaction =~ /^q/i ) { exit; } } } if ( $cash > $goal ) { my $dif = $cash - $goal; notify_user( qq|Yay! you won, you beat your goal by \$$dif\n| ); } if ($cash < $goal) { my $dif = $goal - $cash; notify_user( qq|Boo! you lost, you missed your goal by \$$dif\n| ) +; } sub daily_report { my ($portfolio, $ticker, $cash, $date) = @_; my @report; push @report, qq|\n\nYour Stock Report for January $date\n\n|, qq| Holdings Value\tCompany\n\n|; push @report, qq|\t$$portfolio{$_}\t \$$$ticker{$_}{price}\t$_\n| for keys %$portfolio; push @report, qq|\nYou have \$$cash in cash and \$|, portfolio_value( @_ ), qq| in stocks.\n|; return join '', @report; } sub generate_random { my ($start, $end) = @_; $end = 0 - ($start = 0 - $start) unless defined $end; return int( $start + rand($end - $start + 1) ); } sub portfolio_value { my ($portfolio, $ticker) = @_; my $total_value = 0; $total_value += $$portfolio{$_} * $$ticker{$_}{price} for keys %$p +ortfolio; return $total_value; } sub sell { my ($portfolio, $ticker) = @_; my $cash = 0; my $stock_to_sell = query_user(q|What Stock would you like to Sell +? |); foreach my $stock ( keys %$portfolio ) { if ( $stock =~ /$stock_to_sell/i ) {; if ( $$ticker{$stock}{price} == 0 ) { notify_user( qq|$stock has filed for protection from c +reditors|, q| - Your holdings were liquidated.| ); } else { my $shares_to_sell = query_user( qq|\n\nYou have $portfolio{$stock} shares in $stoc +k\n|, q|Sell How many? :| ); if ( $shares_to_sell > $$portfolio{$stock} ) { notify_user( qq|Sorry, your account is not set up +for short sales.\n| ); } else { $$portfolio{$stock} -= $shares_to_sell; $cash = $shares_to_sell * $$ticker{$stock}{price}; } } last; } } return $cash; } sub buy { my ($portfolio, $ticker, $cash) = @_; my $stock_to_buy = query_user( q|What Stock would you like to buy? + |); foreach my $stock ( keys %$portfolio ) { if ( $stock =~/$stock_to_buy/i ) { if ( $$ticker{$stock}{price} == 0 ) { notify_user( qq|Sorry, trading has been suspended in $ +stock.| ); } else { my $shares_to_buy = query_user( qq|\n\nYou have $portfolio{$stock} shares in $stoc +k and \$$cash\n|, q|Buy How many (max |, int( $cash / $$ticker{$stoc +k}{price} ), q|) :| ); if ( $cash < $$ticker{$stock}{price} * $shares_to_buy +) { notify_user( qq|Sorry, this isn't a margin account +.\n| ); } else { $$portfolio{$stock} += $shares_to_buy; $cash -= $shares_to_buy * $$ticker{$stock}{price}; } } last; } } return $cash; } sub query_user { notify_user(@_) if @_; chomp (my $answer = <STDIN> ); return $answer; } sub notify_user { return print @_; } __END__

We still need to check for partial share sales and to verify some responses, but it should be easier to maintain and update the code with the subroutines. It also makes the main loop very readable. There are some excellent books about programming and perl you should look at. I like to use the public library. I preview most any book for two weeks before I purchase it. Good Luck!




HTH,
Charles K. Clarkson


The great questions are those an intelligent child asks and, getting no answers, stops asking.
- George Wald