'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=\"\">&nbsp;&nbsp;<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=\"\">&nbsp;&nbsp;<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="">
      &nbsp;&nbsp;
      <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