sub-routines

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

I am having trouble with a simple sub-routine that i wrote. All it does is remove some unwanted characters from a file by substituting them with nothing. The problem is that when I call the sub-routine, i assign it to a variable called @derivative. When i print @derivative, i expect it to print the whole of the file, but infact it only prints the last section. Is there an obvious reason for this that I can't see??? Any help vastly appreciated ................



#! /usr/bin/perl -w

use strict;

open (INFILE, $ARGV[0]) or die "unable to open file";
open (OUTFILE, $ARGV[1]) or die "unable to open file";

my $line;
my @derivative = replace($line);

print "@derivative\n";


sub replace {


      $line = @_;

  while (<INFILE>)    {

         $line = $_;
         chomp ($line);
         $line =~ s/data//;
         $line =~ s/=//;
         $line =~ s/detector//;
        
    }

  return $line;

}
[download]

Comment on sub-routines Download Code

Replies are listed 'Best First'.
Re: sub-routines by jreades (Friar) on Nov 28, 2002 at 12:45 UTC
The quick answer is that you only `return $line` and not the entire file, but there are some other significant issues with the code. You also reset the variable `$line` twice within the subroutine, and it's not scoped properly. Although you can sort of get away with what you're doing here (calling a file within a subroutine that isn't properly scoped to that subroutine), it's hard to read and prone to bugs. A better way to think about this is to address the fact that the subroutine operates on a single line at a time, and not the entire file. Don't shoehorn in the file operation as well since it doesn't make logical sense. Instead, pass the subroutine a line at a time since that is its logical 'scope' (as distinct from the lexcial scope). `open (IN "<...") or die("..."); open (OUT ">...") or die("..."); my $line; while ($line = <IN>) { print OUT replace($line); # you could drop the $line and use $_ } close IN; close OUT; exit 0; sub replace { my $line = shift; #skip the comp since you're printing out newlines $line =~ s/data\|=\|detector//g; return $line; }` [download] I think that'll do it.	[reply] [d/l] [select]
Re: sub-routines by LTjake (Prior) on Nov 28, 2002 at 12:50 UTC
Perhaps this is more a point of "style", but to me, your sub replace should either open the file and loop through the contents, or simply replace the text. Openening the file outside the sub, then letting the sub loop through it just feels odd to me. Here's how i might do it: `use strict; open (INFILE, $ARGV[0]) or die "Unable to open file '$ARGV[0]' - $!"; while(<INFILE>) { print replace($_); } close (INFILE); sub replace { my $line = shift; chomp ($line); $line =~ s/data\|=\|detector//g; return $line; }` [download] Tested on this file: `xyzzy data abc detector = data hooplah = enddata` [download] From the command line (note: i opted to use > to print to an OUTFILE rather than do it in the script): `C:\>perl a.pl test.txt > out.txt` [download] Results: `xyzzy abc hooplah end` [download] HTH Update: Fixed some minor things with the code. Also, as you can see, my output is on one line due to the fact that we're chomping each line. You may wish to leave that out if you want to preserve the lines. -- Rock is dead. Long live paper and scissors!	[reply] [d/l] [select]
Re: sub-routines by Chief of Chaos (Friar) on Nov 28, 2002 at 12:37 UTC
Hi, your are using `$line` too much. For each individual scope you should add `my $line;`. `#!/usr/local/bin/perl -w use strict; use warnings; open (INFILE, "<$ARGV[0]") or die "unable to open file"; open (OUTFILE, ">$ARGV[1]") or die "unable to open file"; while (<INFILE>) { my $line = $_; print OUTFILE replace($line); } sub replace { my $line = shift; chomp ($line); $line =~ s/data//; $line =~ s/=//; $line =~ s/detector//; return $line."\n"; } close(INFILE); close(OUTFILE);` [download] Sorry, pasted the wrong statement. Now it is correct.	[reply] [d/l] [select]
Re: sub-routines by Abigail-II (Bishop) on Nov 28, 2002 at 12:54 UTC
Your code doesn't make any sense. You're manipulating all the lines of the input, and then you discard them. Also, your replace subroutine just returns the number of arguments passed. Abigail	[reply]


The stupid question is the question not asked
	PerlMonks