1

I'm having difficulty validating user input with my regex. I'm trying to take user input and make sure it's between 1 and 99. If it's not then it calls the same sub-routine until a validating entry is matched. However, even if the input is between 1-99 it will keep calling the sub routine.

sub get_age
{
    print "Age :";
    @get_age = <STDIN>;
    if (scalar @get_age  !~ m/\d[1-99]/ ) #making sure age given is between 1 and 99
    {

            print "ERROR, invalid input\n";
            &get_age; #this is callng the same sub it's in to re-run validating
    }

            push(@get_age, <STDIN>); #taking name from user and putting it in @get_name     array
            chomp(@get_age); #erasing newline from input
}
Improve this question
2
  • 3
    What's the problem with a simple if condition to test >1 and <99? Commented Apr 7, 2013 at 20:51
  • @Cratylus: The problem with it is a pair of off-by one errors... He'd want to test for >=1 and <=99. Commented Apr 7, 2013 at 21:28

4 Answers 4

Reset to default
4

You seem to be confused about

  1. What the @ sigil does in Perl,
  2. How character classes in regexes work,
  3. What context in Perl is
  4. How the readline operator <...> behaves depending on context.
  5. (Why you should never do &foo).

The @ sigil

The @ denotes an array. This imposes list context on the right hand side of an assignment.

Character classes

A character class is denoted by square brackets, and can include ranges of characters, e.g. [0-9] — match the arabic digits. The \d named character class matches many more characters — whatever Unicode considers "numeric".

A character class can be negated with a leading caret. E.g. [^0-9] matches any character that is not an arabic digit.

Context

Perl operators behave differently depending on context. For example the readline operator:

  • In scalar context (singular), the readline operator reads a single line, but
  • in list context (plural), the readline operator reads all lines until the end of file (EOF) is encountered. To send the EOF on keyboard input, you usually use Ctrl-D.

When an @array is encountered, it evaluates to the list of all elements, in list context. However, in scalar context, it evaluates to the length of the array.

What @get_age = <STDIN> does

Because the LHS is an array, the RHS is evaluated in list context. As explained above, this reads input until the stream is exhausted. Each element in @get_age holds one line of the input after the assignment.

What scalar @get_age !~ m/\d[1-99]/ does

This expression is similar to

my $length = scalar @get_age;
not $length =~ /\d[1-9]/;

I.e. the array in scalar context evaluates to the length of the array, which is the number of lines in the input.

The regex says “Inside the string, there must be one unicode numeric character followed by either 1, 2, 3, 4, 5, 6, 7, 8, or 9”. Multiple occurrences of a character in a charclass are discarded. The string may include arbitrary other characters. This should match: "my age is 020!"

What you probably wanted to do:

sub get_age {
  print "Age: ";
  my $age = <STDIN>; # scalar context, and put into a private variable
  chomp $age;        # remove newline
  if( not length $age ) { # check for empty string
    say "You didn't type anything. Retry!";
    return get_age();
  }
  if( $age =~ /[^0-9]/ ) { # recurse on any non-digits
    say "You typed non-numeric characters. Restrict yourself to '0' through '9'. Retry!";
    return get_age();
  }
  unless( 1 <= $age && $age <= 99 ) { # check for correct bounds
    say "Valid ages have to be in the range 1 to 99. Retry!";
    return get_age();
  }
  return $age;
}

Then in your main code:

my $age = get_age();
Improve this answer
Sign up to request clarification or add additional context in comments.

7 Comments

+1 Love long answers :) 1. Might avoid the recursion and iterate in a loop 2. Why mix && and or ?
@Andomar: re: #2, operator precedence. &&/|| bind relatively tightly (have high precedence), so they're best used for logical operations. and/or bind extremely loosely (low precedence), so they're good for flow control. Basically, mixing them this way helps to ensure that it doesn't get parsed as 1 <= $age && ($age <= 99 || return).
a regex wouldn't have allowed non-integers; I'd guess that isn't a desired feature in your code.
@ysth You are absolutely correct, the updated code should now cover all cases.
Thank you for taking time to list all of this. I am trying to get more practice with regular expressions however, which is why I didn't go through the standard approach of age > 0 && age < 99
|
2 
#!/usr/bin/env perl

use 5.012;
use strict; use warnings;
use Term::Prompt qw( prompt );

my $age = prompt(s => 'Age:', 'between 1 and 99', '', sub {
        my ($input) = @_;
        ($input) = ($input =~ /\A\s*( [1-9] [0-9]? )\s*\z/x);
        return $input and ($input >= 1) and ($input <= 99);
    }
);

say $age;
Improve this answer

Comments

1

[1-99] is the set of all characters from 1 to 9 and the character 9.

Try [1-9]|([1-9]\d) instead. That means either 1-9 or any two numbers not starting with a zero.

Improve this answer

5 Comments

and what about 20, 30, 40, ... ?
@MiguelPrz: [1-9]\d matches 20, 30 and 40. And 50 too.
sorry, I didn't see the \d
and 1-watermelon (1𝟘). if you mean [0-9] say it, not \d (or in 5.14+ use the /a flag)
If you want to only match 0..9 you should specify it; otherwise you will match more than you want perl -E'no warnings;for("\x{FF11}","\x{1D7CF}","\x{1D7D9}","\x{1D7E3}","\x{1D7ED}","\x{1D7F7}","\x{0967}"){say if /^\d$/}'.
1

I think this is an improved version of your code:

use strict;
use warnings;

sub get_age_and_name
{
    my $age;
    while(1) {
        print "Age : ";
        $age = <STDIN>;
        chomp $age;
        # Making sure age given is between 1 and 99
        last if( $age =~ m/^\d\d?$/ and ($age>=1) and($age<=99) );
        print "Error ERROR, invalid input\n";
    } 

    print "Name : ";
    my $name = <STDIN>;
    chomp $name;
    return ($age, $name);
}


my ($age, $name) = get_age_and_name;
print "($age, $name)\n";
Improve this answer

2 Comments

Instead of while(1){...;last if ...; print "error"}, how about {...;if( !... ){ print "error"; redo}}
because TIMTOWTDI,... I've used what it's more readable for me ;)

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.