http://www.perlmonks.org?node_id=587284

Andrew_Levenson has asked for the wisdom of the Perl Monks concerning the following question:

I have a certain script that I have been working with for a while now, watching it evolve as I learn new things. It really doesn't do anything important except for helping me understand new concepts as I implement them.

Today I learned of the ?: operator, and decided to implement it. Now, when I introduce the input and output files and command line arguments, this works perfectly. However, if I don't, and include them as input, it tells me that the argument is invalid and contains a newline.

Can anyone tell me why the newline character is not being removed like it has in all four preceding iterations of this script? (Only the input lines have been changed.)

use strict; use warnings; $_[0]=!@ARGV?<>:shift @ARGV; $_[1]=!@ARGV?<>:shift @ARGV; handlefix(); open(INFILE, '<', $_[0]) or die "\nCan't open $_[0]: $!\n"; open(OUTFILE, '>', $_[1]) or die "\nCan't open $_[1]: $!\n"; print OUTFILE map { my $s=$_; $s=~s/\s*#.*$//; $s } (grep { !/^\s*#/ } <INFILE>), "\n" ; close INFILE && close OUTFILE; sub handlefix { for(@_){ chomp($_); $_=~s/"//g; $_=~s/\//\\/g; } }

Thanks in advanced!

Replies are listed 'Best First'.
Re: Invalid argument
by suaveant (Parson) on Dec 01, 2006 at 20:26 UTC
    Ok.. your use of @_ is just plain wrong, which is your main problem... we'll forgive it since you are learning :)

    You use strict and warnings, that is good.
    You use @_ so strict doesn't complain when you don't declare a variable, that is bad.

    @_ is a special variable that Perl uses to provide you with the arguments that were passed to a subroutine. (which is why strict ignores it). What is happening is that you set @_ to two values, you then call handle_fix. Perl then silently replaces the values in @_ with that arguments passed to the subroutine (empty array), and runs the sub. On return Perl restores the old values of @_ and merrily continues along, making you angry :)

    I would change the code to something more like...

    se strict; use warnings; #why negate, reverse the operations. Also, specifying STDIN is more se +lf documenting. my $infile = @ARGV ? shift(@ARGV) : <STDIN>; my $outfile = @ARGV ? shift(@ARGV) : <STDIN>; handlefix($infile,$outfile); open(INFILE, '<', $infile) or die "\nCan't open $infile: $!\n"; open(OUTFILE, '>', $outfile) or die "\nCan't open $outfile: $!\n"; print OUTFILE map { my $s=$_; $s=~s/\s*#.*$//; $s } (grep { !/^\s*#/ } <INFILE>), "\n" ; #close INFILE && close OUTFILE; close returns a value... if the first +close fails Perl will ignore second close close INFILE; close OUTFILE; sub handlefix { for(@_){ chomp($_); $_=~s/"//g; $_=~s/\//\\/g; } }
    Now... keep in mind iterating through @_ and changing $_ directly WILL change the arguments passed to your sub, but is generally not the best practice. It is not obvious that the args are being changed and later that can bite you or someone else maintaining the code... a better way might be...
    sub handlefix { my @return; for(@_){ my $val = $_; #decouple the value from the argument variable chomp($val); $val =~ s/"//g; $val =~ s/\//\\/g; } return @return; } ($infile,$outfile) = handlefix($infile,$outfile); #more obvious what i +s happening
    Hope that helps

                    - Ant
                    - Some of my best work - (1 2 3)

      Wow, I ask for advice and get an entire mini-tutorial.
      THAT'S why I love it here.

      Thanks for the schooling, I never knew that you could specify variables to be used in a subroutine!
        my and local work in any scope...
        my $test = 1; print "$test\n"; { my $test = 2; print "$test\n"; test(3); } print "$test\n"; sub test { my($test) = @_; # parens make list assignment, otherwise you get the + count of items in @_ print "$test\n"; } #prints... 1 2 3 1
        You can also do something like
        while(my $var = shift @ARGV) { print "$var\n"; # var is scoped only in the while loop }

                        - Ant
                        - Some of my best work - (1 2 3)

Re: Invalid argument
by blokhead (Monsignor) on Dec 01, 2006 at 20:27 UTC
    It looks like you're using a global to pass arguments into handlefix. But the global you decided to use is @_, and Perl's subroutine calling mechanism also uses @_ to pass arguments, so those "global" values are not seen inside the sub. Why not use a normal method of passing arguments to a sub?
    my $infile = !@ARGV?<>:shift @ARGV; my $outfile = !@ARGV?<>:shift @ARGV; handlefix($infile, $outfile);
    You don't even have to change handlefix.

    Might I also suggest to rewrite conditionals to avoid having a negative condition? For example, $infile = @ARGV ? shift @ARGV : <>

    blokhead

      Whoops, I didn't re-think the logic of it.
      I have the negative condition in there because I originally started with two commands, and spent maybe half an hour frustrated because I could not figure out why, even with cmd arguments, it asked me for input.

      I eventually figured out that because I was shifting @ARGV, there was nothing left afterwards. ;)
Re: Invalid argument
by liverpole (Monsignor) on Dec 01, 2006 at 20:19 UTC
    Hi Andrew_Levenson, Yes, it's because you have a newline at the end of your arguments.

    Note what happens when you do:

    print "Debug: Arguments are $_[0] and $_[1]\n";

    Read about chop and chomp for more information on how to remove newlines.

    Here's a better implementation of what you're trying to do (note that chomp only removes the last character if it's a newline):

    chomp(my $arg1 = shift || <>); chomp(my $arg2 = shift || <>);

    s''(q.S:$/9=(T1';s;(..)(..);$..=substr+crypt($1,$2),2,3;eg;print$..$/
      Liverpole -- I know I have a newline in my arguments, but I also have them being chomped in the subroutine. :-\
      That's why i'm so confused.
        Whoops -- didn't read that far :-/

        It's because you're not passing @_.

        You could fix this with:

        handlefix(@_);
        but it's really better practice to chomp the variables on input, and assign them to actual variable names, leaving @_ to its better purpose of the default argument list.

        s''(q.S:$/9=(T1';s;(..)(..);$..=substr+crypt($1,$2),2,3;eg;print$..$/
        Because handlefix(); sends nothing to @_ in your sub.

        Take a look at perlsub, @_ is local

        grep
        XP matters not. Look at me. Judge me by my XP, do you?