Because you've already shifted out ARGV[0] so ARGV[1] is now ARGV[0].

I'd be inclined to:

use strict;
use warnings;

my ($based_on, $top) = @ARGV;
$based_on ||= 'tp';
$top ||= 1;
print "$based_on $top\n";
[download]

which is fine so long as $top can't == 0

my $based_on = $ARGV[0] ? shift: 'tp';
my $top      = $ARGV[0] ? shift: 1;
print "$based_on $top\n";
[download]

Slightly opaque bug there. ;)

shift without args, in main code is the same as shift @ARGV; I'm sure we agree there. Well, when you shift the first value off of @ARGV, that changes the array, so in your second comparison, you're looking at what used to be the third element.

For example:

#perl mytest.pl sn 5

# Before anything else is done, @ARGV looks like:
# $ARGV[0] = 'sn'
# $ARGV[1] = '5'

my $based_on = $ARGV[0] ? shift: 'tp';

# Now @ARGV looks like this:
# $ARGV[0] = '5'
# $ARGV[1] = undef

my $top = $ARGV[1] ? shift: 1;

# It's going to return '1' because $ARGV[1] was undef, due to
# the first shift.
[download]

What you should try instead is:

my $based_on = defined( $ARGV[0] ) ? $ARGV[0] : 'tp';
my $top      = defined( $ARGV[1] ) ? $ARGV[1] : 1;
[download]

...or using shift:

my $based_on = defined( $ARGV[0] ) ? shift : 'tp';
my $top      = defined( $ARGV[1] ) ? shift : 1;
[download]

Your last example has the same bug as the OP. It should be;

my $based_on = defined( $ARGV[0] ) ? shift : 'tp';
my $top      = defined( $ARGV[0] ) ? shift : 1;
[download]


___________
Eric Hodges $_='y==QAe=e?y==QG@>@?iy==QVq?f?=a@iG?=QQ=Q?9'; s/(.)/ord($1)-50/eigs;tr/6123457/- \/|\\\_\n/;print;

How funny, and what irony! :) It goes to show how easy it is to make that kind of mistake if you let your guard down. You're correct, of course, and that's what I intended to say.

Cheers!




Dave

I would write it as:

my $based_on = $ARGV[0] || 'tp';
my $top = $ARGV[1] || 1;
[download]

my $based_on = $ARGV[0] ? shift: 'tp';
my $top      = $ARGV[0] ? shift: 1;#change here
print "$based_on $top\n";

# Then the rest of the code
[download]

The problem is that when use use shift you are removing the first element of the array therefore after the shift the second arg is the first in the @ARGV list.

use Params::Validate;

my ($based_on, $top) = validate_pos(@ARGV, { default => 'tp' }, { defa
+ult => 1 });
[download]

  mycode.pl tp 5
[download]

  mycode.pl 5
[download]

      1 #!/usr/bin/perl -w
      2 
      3 use strict;
      4 
      5 my $based_on = ($ARGV[0] && ($ARGV[0] =~/tp|sn/)) ? shift : 't
+p';
      6 my $top = ($ARGV[0] && ($ARGV[0] =~ /[1-5]/)) ? shift : 1;
      7 scalar(@ARGV) && die "Arguments not parsed: " . join(':', @ARG
+V) . "\n";
      8 
      9 print "$based_on $top\n";
[download]

Oops. I should have ^ and $ in my regexes to force the whole thing to match. -Reenen

Look at all the trival errors people have made in their replies, and you'll see why configuring the program with bare command line arguments is a poor way to do things. If you are writing a quick 10-liner to get something done quickly, it might be reasonable to use this method, but for any longer program, better solutions exist. The best way is to use flags to name the arguments; then you can skip arguments which should use the default value, or specify them in whatever order is convenient or natural at the time, rather than being forced by the hard-coded ordering.

I would suggest using Getopt::Long ... and if this is a longer program, you'll have builtin documentation, of course:

use warnings;
use strict;
use Getopt::Long;
use Pod::Usage;
my %options = ( based_on => 'tp',
                top      => 1
               );
GetOptions( \%options,
            'based_on=s',
            'top=i',
            'man',
            'help',
          ) || pod2usage(2);

pod2usage(1) if $options{'help'};
pod2usage( '-verbose' => 2 ) if $options{'man'};
[download]

Then you invoke your program as:

%> myprog -based_on sn -top 5
# or as 
%> myprog -top 3 -based_on sn
#or you can use the automaticly provided abbreviation recognition ...
%> myprog -base sn -t 2
# or use tht default values
%> myprog
[download]

There are ways to specify default values in the GetOPtions arguments, but I prefer to specify them by initializing %options.

While you are at it, you can specify within the file that the program should be run using Perl. In Unix, make the very first line:

#!/usr/bin/perl
[download]

or whatever the correct path is on your system. There's a similar, two line solution for Windows; I believe it's given in the Perl documention. Type:

perldoc perltoc
[download]

and spend the next three days reading.