Grade me! TAC

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

A total novice wishes to improve his grasp of Perl idioms. I've written a version of TAC from llama, using rob_au's notes about temporary files. I'm trying to do it without any slurping. Anyone like to critique this?

# This is TAC
# reverse of cat
#
# Take a list of files as command line parameters
# and display the files on STDOUT
# last file first
# displaying each file backwards

# The files are concatenated in 'normal' order to a temporary file
# which is then read and displayed in reverse

# check the number of parameters..


my $argCount = $#ARGV+1;
if ($argCount<2)
    {
    exit 0;
    }
# open the temporary file
use Fcntl;
use POSIX;
my $outputFile;
do {
    $outputFile = tmpnam();
} until sysopen(OUTPUTFILE, $outputFile, O_RDWR|O_CREAT|O_EXCL, 0666);


# for every file..
while (@ARGV)
    {
    # open it

    my $fileName = shift;
    open ( MYFILE, " $fileName" ) or die "Cant open file $fileName";
    # and write each line to the temporary file..

    foreach (<MYFILE>)
        { print  OUTPUTFILE $_; }
    close MYFILE;
    print OUTPUTFILE "\n";
    print "Done $fileName -\n";
    }
close OUTPUTFILE;

# now read it in backwards..
my $inputFile = $outputFile;
open ( INPUTFILE, "< $inputFile");
my $char;
my $fromEnd=1; 
my $fileSize = (stat INPUTFILE)[7];
# we seek to a position a distance from the end, and read a single cha
+racter
# the distance from end increases till we hit the start
while ($fromEnd<$fileSize+1)
    {
    seek INPUTFILE, -$fromEnd, 2; # last char
    read INPUTFILE, $char, 1;
    print $char;
    ++$fromEnd;
    }


close INPUTFILE;
unlink $inputFile;
[download]

Comment on Grade me! TAC Download Code

Replies are listed 'Best First'.
Re: Grade me! TAC by Joost (Canon) on Jun 11, 2004 at 11:01 UTC
Why are you using a temporary file at all? Couldn't you just read backward from each file and print to STDOUT? something like `for my $file (reverse @ARGV) { read_backward_and_print($file); }` [download] Why do you need 2 arguments? I'd guess you might want to reverse a single file too... I'd write your check as: `die "Usage: $0 FILENAMES\n" unless @ARGV >= 2;` [download] Also, I was under the impression that `tac` was supposed to print the lines in the files in reverse order, NOT to also print the reverse of each line... "What should it profit a man, if he should win a flame war, yet lose his cool?"	[reply] [d/l] [select]
Re^2: Grade me! TAC by Frank_Zappa_lives (Initiate) on Jun 11, 2004 at 14:19 UTC
temporary file? It kind of grew that way. I can see I dont need it now. 2 args? well i just thought there is no concatenattion then reverse of each line? well, not a lot different. :-)	[reply]
Re: Grade me! TAC by gri6507 (Deacon) on Jun 11, 2004 at 13:16 UTC
Just a a general comment on perl coding style, try to make use of the $! variable in errors like this, `open ( MYFILE, " $fileName" ) or die "Cant open file $fileName: $!\n";`	[reply] [d/l]
Re^2: Grade me! TAC by davido (Cardinal) on Jun 11, 2004 at 15:45 UTC
Ok, I'll bite. Why? \n requires simple double-quotish interpolation to work. $/ requires double-quotish interpolation, plus variable lookup in the symbol table. $/ can also be re-set by the script to some other value (other than \n). If the script sets $/ to "End of Record", and you use $/ in place of \n... well, you know what it's going to print. I don't mind seeing $/ here and there in one-off scripts, but can't imagine why you would use it to represent \n in a script that's going to be around awhile. It's definately not something you would read in perlstyle. Maybe I missed the point? UPDATE: Yes, I missed the point. The italics font made $! look like $/. Sorry everyone. :) Dave	[reply]
Re^3: Grade me! TAC by runrig (Abbot) on Jun 11, 2004 at 16:05 UTC
Look closely. grid6507 is advocating the use of dollar-bang, not dollar-slash. (Though it does look kind of like a slash in italics, doesn't it?).	[reply]
Re^4: Grade me! TAC by davido (Cardinal) on Jun 11, 2004 at 16:08 UTC
Re: Grade me! TAC by bart (Canon) on Jun 11, 2004 at 10:53 UTC
You might want to take a look at File::ReadBackwards.	[reply]
Re: Grade me! TAC by FoxtrotUniform (Prior) on Jun 11, 2004 at 16:36 UTC
You get a gold star for soliciting a code reveiw. Getting people to look over one's code is a real help to learning a new language. You don't have any subs in this script? Okay, it's only 35 lines or so of actual code, but it could still be broken up into more logical chunks. I'd write subs for tempfile creation, appending a file to the tempfile, and printing a file backwards. I see no "strict" or "warnings" here. You'll want those. `-- F o x t r o t U n i f o r m Found a typo in this node? /msg me % man 3 strfry`	[reply]
Re: Grade me! TAC by thens (Scribe) on Jun 12, 2004 at 12:08 UTC
`my $argCount = $#ARGV+1;` To find the number of elements in an array, use the array in scalar context. `my $argCount = @ARGV;` To find out how this is different (and why it is the correct way) read about the variable `$[` by doing a `perldoc perlvar` The sample code will illustrate that. `use strict; $[ = 2; my @num = ( 1..4 ); print "No of elements >> ", $#num+1, "\n"; print "No of elements >> ", scalar @num, "\n";` [download] Ouput No of elements >> 6 No of elements >> 4 `$#` is actually the last subscript of the array and the variable `$[` sets the base subscript to 2 (default is zero). To access the elements in `@num` you will have to use subscripts 2,3,4,5 and that is the reason you get '6' in the first case (5+1) and not '4' which is the actual number of elements. Agree that when you dont reset `$[` both the methods are same. Why use a complex last subscript+1 calculation when there is an easier way to find the number of elements that works in all cases ?? -T	[reply] [d/l] [select]