Silly While Loop Problems

[gsus]

Evening Perl Monks

I have a really simple requirement on a project that i'm working on at home which requires what i believe is a while loop. In it's most simple form, i want to prompt a user running a script to enter either Y or N to continue or abort the script.

Below is what i thought might work, oh and i have this in a subroutine.

sub continue_yn
  {
   print "Do you wish to continue the installation (Y/N) : ";
   $yn=<STDIN>;

   while (lc($yn) ne "y" || lc($yn) ne "n")
     {
      print "Invalid option, please enter an Y or N : ";
      $yn=<STDIN>;
     }

   print "Valid option entered";
  }

When my program calls this sub routine, no matter what i enter in for the variable $yn it never tests TRUE in the while loop and just continues to prompt me with my Invalid Option message.

Sorry i should have mentioned that once i've gone through the loop i'll have an if statement to either exit and abort the program or just drop out of the sub routine and continue processing.

Thanks

[MidLifeXis]

Time to go look at your truth tables, even after taking into account the newline.

   while (lc($yn) ne "y" || lc($yn) ne "n")

lc($yn)	lc($yn) ne "y"	lc($yn) ne "n"	lc($yn) ne "y"||lc($yn) ne "n"
y	false	true	true
n	true	false	true
*	true	true	true

I think you mean AND, not OR.

[gsus]

Hmmmmm, don't i feel stupid :-)

Thanks for the quick responses guys and help.

[blue_cowdawg]

When my program calls this sub routine, no matter what i enter in for the variable $yn it never tests TRUE in the while loop

  $yn=<STDIN>; 
  chomp ($yn);

I also noticed you're not running use strict; or it would have complained about the lack of my $yn=<STDIN>;

I'm just saying...

[roboticus]

gsus:

Try reading perldoc -f chomp. Many also suggest the use of s/\s+$//; to remove all trailing whitespace.

...roboticus

When your only tool is a hammer, all problems look like your thumb.

[Anonymous Monk]

Thanks guys but chomp didn't solve my problem, the strange thing is that if i put a print "$yn"; in the loop it prints the input just fine.

Do you wish to continue the installation (Y/N) : y
y
Invalid option, please enter an Y or N : n
n
Invalid option, please enter an Y or N : Y
Y
Invalid option, please enter an Y or N : N
N
Invalid option, please enter an Y or N :

[SuicideJunkie]

Don't do print "$yn";.

Do print "##$yn##\n"; or similar instead. That lets you see whitespace characters

[ikegami]

If you did chomp, it would look like

Do you wish to continue the installation (Y/N) : y
yInvalid option, please enter an Y or N : n
nInvalid option, please enter an Y or N : Y
YInvalid option, please enter an Y or N : N
NInvalid option, please enter an Y or N :

You've actually proved blue_cowdawg found one of the problems.

[ikegami]

If you did chomp, it would look like

Do you wish to continue the installation (Y/N) : y
yInvalid option, please enter an Y or N : n
nInvalid option, please enter an Y or N : Y
YInvalid option, please enter an Y or N : N
NInvalid option, please enter an Y or N :

You've actually proved blue_cowdawg found one of the problems.

[hbm]

For a simple exchange, I don't see the value in rewording the question following an invalid response. I just do this:

   do { 
      print "Question (Y/N)? ";
      chomp($_ = <>);
   } until /^[YN]$/i;

[TomDLux]

If the input were 'y', the first part of the || would be false, but the second part would be true. For an input of 'n', the first part is true while the second is false. Any other input, both parts are true. The only way to avoid the loop if the input is simultaeneously 'y' and 'n'. Only Damian can do that.

I would use shortcut returns to drastically simplify the condition:

sub continue_yn
  {
   print "Do you wish to continue the installation (Y/N) : ";
   $yn=<STDIN>;
   chomp $yn;

  LOOP:
   while ( 1 )
     {
      last LOOP if lc($yn) eq 'y';
      last LOOP if lc($yn) eq 'n';
      print "Invalid option, please enter an Y or N : ";
      $yn=<STDIN>;
      chomp $yn;
     }

   print "Valid option entered";
  }

Or use a hash to specify the set of valid inputs.

Readonly my %VALID_CHAR => ( y => 1, Y => 1, n => 1, N => 1 );
sub continue_yn
  {
   print "Do you wish to continue the installation (Y/N) : ";
   $yn=<STDIN>;
   chomp $yn;

  LOOP:
   while ( 1 )
     {
      last LOOP if $VALID_CHAR{$yn};
      print "Invalid option, please enter an Y or N : ";
      $yn=<STDIN>;
      chomp $yn;
     }

   print "Valid option entered";
  }

WARNING: not tested. Provided for concepts, not cut-and-paste.

[Marshall]

There are lots of ways to do this.

Below, ask_yes_no("prompt XXX") keeps prompting with "prompt XXX" until either a 'Y' or a 'N' answer happens or a quit/abort input is seen which causes the program to completely stop.

A non-answer (blank line) causes just a re-prompt (that's not an error as per standard CLI).

An invalid answer causes an error message and then the re-prompt.

Of course it would be possible to tighten the while loop up so much that it only allows 'Y' or 'N' for the $answer.

The basic idea when prompting the user for an answer:
1) ask the question
2) a "non answer" like a bunch of blanks is ok, just re-prompt
3) an "illegal answer".. something that is clearly wrong should result in an error message and then the re-prompt
4) spaces before or after the "answer" should be meaningless.

In the code below, there are a number of lines of "boiler plate" stuff along the way to returning a Y or a N. It is not necessary to reduce this to the minimum number of lines of code. The computer is ridiculously fast compared with the human being. And sometimes fewer lines of source code does not mean a faster program.

I would advocate something like the below. Ask the question and if the subroutine returns, it has a Y or a N. If it cannot come back with one of those 2 answers, then it should not "come back" to the main program.

#!/usr/bin/perl -w
use strict;

my $answer = ask_yes_no ("Gimme a Yes or No!(Y/N)?");
print "answer = $answer\n";

sub ask_yes_no
{
    my $prompt = shift;
    
    while ( (print $prompt), 
            (my $answer = <STDIN>) !~ /^\s*q(uit)?\s*$|^\s*abort\s*?/i
+   
          )
    {
       next if $answer =~ /^\s*$/;  # re-prompt on blank line
       
       $answer =~ s/^\s*//;  #no leading spaces
       $answer =~ s/\s*$//;  #no trailing spaces
       $answer = uc $answer;  
       return $answer if ($answer eq 'Y' or $answer eq 'N');
       
       print "Illegal entry try again..!!\n";       
    }
    
    die "quit or abort was entered! **Abort Program!!**\n";    
}

__END__
Example: UI test run....

C:\TEMP>perl askyn.pl
Gimme a Yes or No!(Y/N)?2345
Illegal entry try again..!!
Gimme a Yes or No!(Y/N)?y
answer = Y

C:\TEMP>perl askyn.pl
Gimme a Yes or No!(Y/N)?N
answer = N

C:\TEMP>perl askyn.pl
Gimme a Yes or No!(Y/N)?a
Illegal entry try again..!!
Gimme a Yes or No!(Y/N)?ABORT
quit or abort was entered! **Abort Program!!**

C:\TEMP>perl askyn.pl
Gimme a Yes or No!(Y/N)?Q
quit or abort was entered! **Abort Program!!**

C:\TEMP>perl askyn.pl
Gimme a Yes or No!(Y/N)?
Gimme a Yes or No!(Y/N)?
Gimme a Yes or No!(Y/N)?
Gimme a Yes or No!(Y/N)?

[OfficeLinebacker]

Try testing just the first character of $yn.