'lo
Andrew Wilson
andrew at rivendale.net
Mon Aug 27 08:12:38 CDT 2001
On Mon, Aug 27, 2001 at 11:35:49AM +0100, barry wrote:
> On Mon, Aug 27, 2001 at 11:29:08AM +0100, Patrick Moore mumbled:
> > hullo all,
> > finally got around to joining the pm list, so I thought I'd say hello.
> > "Hello!"
>
> hiya! of course this is my first post here too, amazing how many recognisable
> names you see around, anyway, to keep this at least a bit perl related, I got
> bored yesterday and ran code2html on a bunch of my really sloppily written
> code snippits and shoved them at http://bazza.com/code/ for people to go and
> poke fun and laugh at
Hi bazza,
Well, since you've asked ... I've had a look a photos.html
#!/usr/bin/perl -w
##################
&static;
foreach(<*>) {
next if($_ =~ /~/);
next if($_ =~ /cgi/);
if($_ =~ /jpg/) {
print "<tr><td bgcolor=#EEEEEE><font face=\"Arial, Helvetica\"><img border=0 src=\"/icons/image2.gif\" alt=\"\"> <a href=\"$_\">$_</A></td>\n";
$commentfile = ".$_-comment";
if (-e $commentfile) {
open(COMMENT, ".$_-comment");
$comment = <COMMENT>;
print "<td><font face=\"Arial, Helvetica\">$comment</td></tr>";
close(COMMENT);
}
} else {
print "<tr><td bgcolor=#EEEEEE><font face=\"Arial, Helvetica\"><img border=0 src=\"/icons/folder.gif\" alt=\"\"> <a href=\"$_\">$_</A>/</td>\n";
$commentfile = ".$_-comment";
if (-e $commentfile) {
open(COMMENT, ".$_-comment");
$comment = <COMMENT>;
print "<td><font face=\"Arial, Helvetica\">$comment</td></tr>";
close(COMMENT);
}
}
}
&end;
sub static {
print <<END;
Content-Type: text/html <html>
<head><title>Photos</title></head>
<body bgcolor=#FFFFFF>
<table>
<tr><td bgcolor=#EEEEEE><font face="Arial, Helvetica"><a href="../">../</a></td>\n<td><font face="Arial, Helvetica">Previous Dir</td></tr>
END
} sub end {
print <<END;
</table>
<center><a href="http://bazza.com/"><img src="/bazza.com-icon.jpg" border=0></a><center>
</body>
</html>
END
}
Some of these are minor niggles, this one isn't you havent used
strict. that's almost always a mistake.
You don't name the variable that you use to control the loop but then
you dont take proper advantage of $_ either.
either do
foreach (<*>) {
next if /~/;
next if m#.cgi$#;
if (m#.jpg#) {
...
or do this
foreach my $file (<*>) {
next if $file =~ /~/;
next if $file =~ m#.cgi$#;
if ($file =~ m#.jpg#) {
...
I don't like your regular expressions they match too much. For
instance /cgi/ matches map_of_dirs_containg_cgi_files.jpg and lots of
other things that you don't really want.
/\.cgi$/ is a much tighter and hence much less likely to false match
regexp.
same with /jpg/, should be /\.jpg$/ or if you don't like /\ use
m instead m#.cgi$# and m#.jpg$#
This is a style thing and you can feel free to ignore it. I don't
like long lines like the ones you have in your HTML. You can break
the HTML on whitespace and I think that makes it easier to read.
you aren't taking proper advantage of perls quoting operators either.
You have a habit of doing "I want quotes in here so I'll do this \""
when what you should be doing is
qq{I want quotes in here so I'll do this "}
There is no real need for the subroutines unless you put them in some
other file and import them. Much better to use a templating system
have a look at Template toolkit, it's excellent.
This is definitely a problem, you have identical code in both halves
of the if/else statement. You should extract it. Also you weren't
closing the <tr> tag if there was no comment.
I've refactored it a bit below including the comments I made. As you
can see I've eliminated the if/else statement altogether, ther was
only one word of difference in the two sections. For things like that
you can use the ternary operator
my $src = ($file =~ m#.jpg$#) ? "image2" : "folder";
if $file matches m#.jpg$# then $src is set to "image2" otherwise it's
set to "folder".
#!/usr/bin/perl -w
##################
use strict;
print <<END;
Content-Type: text/html <html>
<head><title>Photos</title></head>
<body bgcolor=#FFFFFF>
<table>
<tr>
<td bgcolor=#EEEEEE>
<font face="Arial, Helvetica">
<a href="../">../</a>
</td>
<td>
<font face="Arial, Helvetica">Previous Dir
</td>
</tr>
END
foreach my $file (<*>) {
next if $file =~ /~/;
next if $file =~ m#.cgi$#;
my $src = ($file =~ m#.jpg$#) ? "image2" : "folder";
print qq{
<tr>
<td bgcolor=#EEEEEE>
<font face="Arial, Helvetica">
<img border=0 src="/icons/$src.gif" alt="">
<a href="$file">$file</a>
</td>};
my $commentfile = ".$file-comment";
if (-e $commentfile) {
open(COMMENT, ".$file-comment");
my $comment = <COMMENT>;
close(COMMENT);
print qq{
<td>
<font face="Arial, Helvetica">$comment
</td>
};
}
print qq{</tr>\n};
}
print <<END;
</table>
<center>
<a href="http://bazza.com/">
<img src="/bazza.com-icon.jpg" border=0></a>
<center>
</body>
</html>
END
Hopefully you found that useful.
cheers
Andrew
More information about the Belfast-pm
mailing list