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

Repeating an input prompt

by zod (Scribe)
on Nov 28, 2008 at 21:01 UTC ( #726670=perlquestion: print w/ replies, xml ) Need Help??
zod has asked for the wisdom of the Perl Monks concerning the following question:

I'm wondering if this is the correct way to repeat an input prompt. Setting a global $flag feels wrong. Anyway, this code works, but if any monk could critique this code, I'd be grateful.

Thanks,

zod

use strict; use warnings; my @positions = qw(sitting standing reclining); my @locations = ('at the museum', 'in the woods', 'on the beach', 'in the kitchen'); my @drinks = qw(scotch beer coke yoohoo); my $flag = 1; do { my $selection = print_options( \@drinks ); $flag = do_it( $drinks[ $selection - 1 ], \@positions, \@locations ); } until $flag == 0; sub print_options { my $drinks_ref = shift; for ( my $i = 0 ; $i <= $#{$drinks_ref} ; $i++ ) { print $i + 1, "\. $drinks_ref->[$i]\n"; } print "Choose a number: "; chomp( my $selection = <> ); return $selection; } sub do_it { my $drink = shift; my $positions_ref = shift; my $locations_ref = shift; my $rand_position = $positions_ref->[ rand( $#{$positions_ref} ) ]; my $rand_location = $locations_ref->[ rand( $#{$locations_ref} ) ]; print "You just drank a $drink $rand_location in the $rand_position position +.\n"; if ( $rand_location eq 'on the beach' ) { print "YEEEEEHAAAWWWWWWWWWW!\n"; return 0; } else { return 1; } }

Comment on Repeating an input prompt
Download Code
Re: Repeating an input prompt
by DStaal (Chaplain) on Nov 28, 2008 at 21:43 UTC

    The multiple functions are probably overkill for this, but in general this looks decent. (I'd use a while loop instead of a do { } unless, but that is partly taste.)

    '$flag' isn't a global variable (conceptually): it's a flag variable who's scope includes the while loop it controls. The fact that in this case that means it is at the top-level scope isn't really relevant. The only real way to write this without the variable would be to have do_it be in the loop condition itself, and that's more ugly. (And less expandable.)

    I'd be tempted to rename '$flag' to '$again': Then do_it returns whether we want to do it again. But that's just me reading it and having fun. ;)

    Oh, and this:

    my $drink = shift; my $positions_ref = shift; my $locations_ref = shift;

    Can be shortened to this:

    my ($drink, $positions_ref, $locations_ref) = @_;
Re: Repeating an input prompt
by AnomalousMonk (Abbot) on Nov 28, 2008 at 21:49 UTC
    Anyway, this code works ...
    An expression like $positions_ref->[ rand( $#{$positions_ref} ) ] will never evaluate to the last element of the array. Use $positions_ref->[ rand( @{$positions_ref} ) ] instead (unless you want to avoid the last element!).
    >perl -wMstrict -le "my $n = shift || 1000; my $arrayref = [ qw{ a b c d e f g i j k l m n o p } ]; my $last_element = $arrayref->[-1]; my $n_max_i = 0; for (1 .. $n) { my $rand_element = $arrayref->[ rand $#$arrayref ]; ++$n_max_i if $rand_element eq $last_element; } printf 'last element picked randomly %f%% of %d times', $n_max_i/$n*100, $n; " last element picked randomly 0.000000% of 1000 times >perl -wMstrict -le "my $n = shift || 1000; my $arrayref = [ qw{ a b c d e f g i j k l m n o p } ]; my $last_element = $arrayref->[-1]; my $n_max_i = 0; for (1 .. $n) { my $rand_element = $arrayref->[ rand @$arrayref ]; ++$n_max_i if $rand_element eq $last_element; } printf 'last element picked randomly %f%% of %d times', $n_max_i/$n*100, $n; " last element picked randomly 6.700000% of 1000 times
    Updates:
    1. Changed examples to use array references.
    2. Changed printf statements to correctly calculate percentage.
      Anyway, this code works ...

      I knew I'd regret saying that. Thanks for pointing out the error.

Re: Repeating an input prompt
by Tanktalus (Canon) on Nov 28, 2008 at 22:05 UTC

    Two points to critique. First, as is pointed out, $flag isn't a global variable once you discount the fact that you're using global code. Second, don't re-invent wheels. Download and install IO::Prompt, and then you can do something like this:

    #!/usr/bin/perl use strict; use warnings; use IO::Prompt; sub main { my @positions = qw(sitting standing reclining); my @locations = ('at the museum', 'in the woods', 'on the beach', 'in the kitc +hen'); my @drinks = qw(scotch beer coke yoohoo); my $flag = 1; do { my $selection = prompt -p => 'The bartender asks what you want:', -onechar => 1, -menu => \@drinks; $flag = do_it( $selection, \@positions, \@locations ); } until $flag == 0; } sub do_it { my $drink = shift; my $positions_ref = shift; my $locations_ref = shift; my $rand_position = $positions_ref->[ rand( @$positions_ref ) ]; my $rand_location = $locations_ref->[ rand( @$locations_ref ) ]; print "You just drank a $drink $rand_location in the $rand_position +position.\n"; if ( $rand_location eq 'on the beach' ) { print "YEEEEEHAAAWWWWWWWWWW!\n"; return 0; } else { return 1; } } main;
    Note how I took your global code, pushed it into a sub, and then called it. Now your $flag isn't global. And now you have to pass the positions and locations refs into do_it, whereas before, it wasn't strictly required (due to the global nature of the variables).

    I've already taken into account Anomolous Monk's observance on the usage of rand.

      Please no! IO::Prompt is a module that TheDamian only wrote for his PBP book and it just doesn't work on Win32 and never can, because it does stupid tricks with devices that don't exist on Win32. It has no test suite, it has lots of serious bugs, it has the usual support by TheDamian, that is, none.

Re: Repeating an input prompt
by hangon (Deacon) on Nov 28, 2008 at 22:09 UTC

    If you really want to get rid of $flag, you could do something like this:

    while (do_it($drinks[print_options(\@drinks) -1], \@positions, \@locat +ions)){}
Re: Repeating an input prompt
by jwkrahn (Monsignor) on Nov 28, 2008 at 22:14 UTC
    my $rand_position = $positions_ref->[ rand( $#{$positions_ref} ) ]; my $rand_location = $locations_ref->[ rand( $#{$locations_ref} ) ];

    You have an off-by-one error on those array subscripts.   'in the kitchen' will never be chosen from @locations and 'reclining' will never be chosen from @positions.

    I would write it more like this:

    #!/usr/bin/perl use strict; use warnings; my @positions = qw( sitting standing reclining ); my @locations = ( 'at the museum', 'in the woods', 'on the beach', 'in + the kitchen' ); my @drinks = qw( scotch beer coke yoohoo ); 1 while do_it( print_options( \@drinks ), \@positions, \@locations ); sub print_options { my $drinks_ref = shift; for my $i ( 0 .. $#$drinks_ref ) { print $i + 1, ". $drinks_ref->[$i]\n"; } my $selection; do { print 'Choose a number: '; chomp( $selection = <STDIN> ); } until $selection >= 1 && $selection <= @$drinks_ref; return $drinks_ref->[ $selection - 1 ]; } sub do_it { my ( $drink, $positions_ref, $locations_ref ) = @_; my $rand_position = $positions_ref->[ rand @$positions_ref ]; my $rand_location = $locations_ref->[ rand @$locations_ref ]; print "You just drank a $drink $rand_location in the $rand_positio +n position.\n"; if ( $rand_location eq 'on the beach' ) { print "YEEEEEHAAAWWWWWWWWWW!\n"; return 0; } return 1; }
      Thanks for the critique. I don't quite understand this line, though:
      until $selection >= 1 && $selection <= @$drinks_ref;
      Wouldn't that always be true if you input 1 - 4 ? Can you elaborate on that a little? thanks.

        I put that in to verify that the selection was a number in the range 1 - 4.

Log In?
Username:
Password:

What's my password?
Create A New User
Node Status?
node history
Node Type: perlquestion [id://726670]
Approved by ww
help
Chatterbox?
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others chilling in the Monastery: (7)
As of 2014-12-18 00:56 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    Is guessing a good strategy for surviving in the IT business?





    Results (41 votes), past polls