tags:

views:

286

answers:

5

When I print the result of the regex I am attempting to use to control the until loop it gives me the 1 or null I am expecting. Why is it that the code below won't work but if I uncomment the fifth line it works fine?

print("Please enter 1, 2, 3 or 4 : ");
my $channelSelection = "";

until ($channelSelection =~ /^[1-4]$/) {
    chomp(my $channelSelection = <STDIN>);
    #last if ($channelSelection =~ /^[1-4]$/);
    print ("Invalid choice ($channelSelection) please try again: ") 
        if ($channelSelection !~ /[1-4]/);
}

I'm sure this has been solved elsewhere but was unable to find it with search. Pointing me in the right direction would be great.

I would normally do something like.

print("Please enter 1, 2, 3 or 4 : ");
my $channelSelection = "";
while (1) {
    chomp(my $channelSelection = <STDIN>);
    last if ($channelSelection =~ /^[1-4]$/);
    print ("Invalid choice ($channelSelection) please try again: ") if ($channelSelection !~ /[1-4]/);
}

But I'm trying to get away from the infinite loops.

+10  A: 

You have redeclared $channelSelection locally within the until loop. That way, its value will be lost every time the loop executes. So the regular expression will not match as the then value of $channelSelection will again be equal to "".

Removal of my from within the loop will solve the issue.

Alan Haggai Alavi
Heh, same answer, 6s apart. :)
Artem Russakovskii
Yes i wanted to give you the check but Artem was ever so slightly faster. hehe Good job tho +1
Copas
That is nice! :-)
Alan Haggai Alavi
+16  A: 

The problem here is you're re-declaring the $channelSelection within the loop but the outside of the loop keeps the old value. Remove the "my" from the inner loop.

Artem Russakovskii
+6  A: 

How about not worrying about it?

#!/usr/bin/perl

use strict;
use warnings;

use Term::Menu;

my @channels = qw( 1 2 3 4 );

my $prompt = Term::Menu->new(
    aftertext => 'Please select one of the channels listed above: ',
    beforetext => 'Channel selection:',
    nooptiontext =>
        "\nYou did not select a valid channel. Please try again.\n",
    toomanytries =>
        "\nYou did not specify a valid channel, going with the default.\n",
    tries => 3,
);

my $answer = $prompt->menu(
    map { $_ => [ "Channel $_" => $_ ] } @channels
);

$answer //= $channels[0];

print "$answer\n";

__END__
Sinan Ünür
This is a cleaner solution by all means, as long as the OP has access to installing modules - Term::Menu is not part of standard Perl dist. Nice assist, however.
Artem Russakovskii
Great solution unfortunately this code needs to run on HP-UX 10.2 running perl 5.004_04. To make matters worse there is pretty much no way to update any of this (much less install a module). In the industry I work in we are constantly fighting to get ANY upgrades to anything that's not 1950's technology.
Copas
@Artem well, the OQ had already been answered, so I wanted to show an alternative. BTW, http://perldoc.perl.org/perlfaq8.html#How-do-I-keep-my-own-module%2flibrary-directory%3f might be useful.
Sinan Ünür
@Copas I feel for you. Arrgh!
Sinan Ünür
@Artem Russakovskii, please don't introduce this idea to people, they generally *can* install modules and you shouldn't be scaring them into thinking they cant which they already tend to assume, its a rare case when you actually can't. http://stackoverflow.com/questions/755168/perl-myths/755179#755179
Kent Fredric
Indeed, if the module is just code, it's fine to keep it local. You won't benefit from cpan upgrade and the path will need to be referenced explicitly in other projects but you're right. Reuse code, people!
Artem Russakovskii
+3  A: 

The best solution for getting input from user is to use IO::Prompt module. It supports repetitions, validations, menu system and much more.

depesz
There seem to be portability issues pointed out by the module reviewers - be careful of those.
Artem Russakovskii
+2  A: 

This is more of a style issue (and since you can't install modules, it doesn't help you), but I just wanted to point out that when checking for fixed values, using a regex is probably not the best solution.

This is is what I would do:

use List::MoreUtils;

my @allowed_values = qw( 1 2 3 4 );

# get $answer from prompt.

if(any { $_ == $answer } @allowed_values) {
    # All is good.
}

Might come in handy some other time.

Anon
I agree this appears to be a much better way to do it (if only). Is there a better way to do it in pure perl 5.004? Thanks +1 for showing a cool mod.
Copas
I guess you could lift the function from the module and check if it works. It's just a few lines and doesn't appear to do any magic..
Anon