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

This is the first thing I've written apart from little tutorial excercises, and it was suggested that I post this to SoPW for further advice, so here goes...

All this does is take an HTML file (or any file, really), wrap an ASP Response.Write command around each line, and dump it back out to a file. Nothing fancy, but I'd like to know if there's something I could be doing better.

($infile, $outfile, $indent) = @ARGV;

open(IN, $infile) or die "Can't open input file: $!";
open(OUT, ">" . $outfile) or die "Can't open output file: $!";

print "Starting...";
print OUT "<%\n";

while(<IN>) {
    chomp;
    print OUT (" " x $indent) . "Response.Write(";
    if ($_) {
        print OUT "\"$_\" + ";
    }
    print OUT "vbNewLine)\n";
     
}

print OUT "%>\n";
print "Done.";

close IN or die "Can't close input file: $!";
close OUT or die "Can't close output file: $!";

[download]

And thanks to the chatterbox folks for helping me with the most egregious errors!

Replies are listed 'Best First'.
Re: A simple input/output script, suggestions wanted
by premchai21 (Curate) on Apr 11, 2001 at 22:16 UTC
    A few hints: 
    
my ($infile, $outfile, $indent) = @ARGV;
# Use my for better scoping

$infile ||= "default_infile";
# For a default value, in case none is supplied;
# if the value is required, die if it doesn't exist;
$outfile ||= "default_outfile";
$indent ||= "20";

# Rather than using the order of arguments,
# you might like Getopt::Long instead.
# It gives you good option-checking, and
# dies if any options are missing or otherwise ungood.

open(IN, $infile) or die "Can't open input file: $!";
# Good.  You're checking the returns of every system call.
open(OUT, ">$outfile") or die "Can't open output file: $!";
# A little easier to read with variable interpolation

print "Starting...";
select (OUT);
# So you don't have to type the filehandle name all the time
print "<%\n";

$indent = ' ' x $indent;
# To save the time required to recompute this;
# if you're using the value of $indent later, just say
# length($indent), or save this in a separate variable

print  "${indent}Response.Write(",($_ ? "$_ +" : ""),"vbNewLine)\n" wh
+ile(chomp ($_ = <IN>));
# Using var interpolation and quick if (?:),
# you can combine all this into one print
# statement; chomp can be put inside the while
# condition; then, since you have a one-statement
# while loop, you can use the alternate syntax
# to avoid messy braces

print "%>\n";
print STDOUT "Done.";
# You could also do select(STDOUT); first

close IN or die "Can't close input file: $!";
close OUT or die "Can't close output file: $!";

    [download]
    See also: perlfunc, perlsyn, perlop, and Getopt::Long.
[reply]
[d/l]
Re: A simple input/output script, suggestions wanted
by suaveant (Parson) on Apr 11, 2001 at 22:24 UTC
    A few little things...
    You could write 
    print OUT "\"$_\" + ";
#as
print OUT qq`"$_" + `;

    [download]
    for better readability... perl allows you to do q<char>text<char> and qq<char>text<char> (and qx and qr) to use any character as you quote delimiter, to negate the need for escaping quotes... not that that is a big deal. qq is for " and q is for '

    You could do 

    print OUT qq`"$_" + ` if $_;
#or 
$_ && print OUT qq`"$_" + `;

    [download]
    but again... that is just cosmetic.
    In general, though... it's really not that bad.

    Oh.. one thing... it would be faster to use a , instead of a . in the print, since the . cocatenates the strings, then prints, whereas the comma just prints them in order, without wasting time concatenating
                    - Ant

[reply]
[d/l]
[select]
Re (tilly) 1: A simple input/output script, suggestions wanted
by tilly (Archbishop) on Apr 12, 2001 at 06:59 UTC
    You need to put the name of the file in your error checks.

    It really helps...

[reply]

Back to Seekers of Perl Wisdom