Offer tips, comments on this code (generates html to index image files)

David Oswald · David Oswald

Please offer suggestions, comments, and tips on the following perl code. I
had a need to quickly generate an html document indexing and displaying all
image files within a given directory.

To use:
Save the script to somewhere within your $path. CD into any directory
containing a series of image files. execute the script (no command line
parameters). It scans the directory, and outputs an html document with img
src tags for each image, and a href tags for links to each image. If you
click on one of the links (from the html document), the image will be
displayed by itself in a separate popup window on a web browser.

This was my first perl project. Necessity is the mother of invention. I
"overwrote" it to make it easy to modify in the future. I'd like to hear
comments:

---------------------------------------

#!/usr/bin/perl

# imgdir2html.pl v1.02 (c)David Oswald
# Send comments to [email]davido@pacifier.com[/email]

# The purpose of this little script is to parse a single directory, reading
in all
# files named *.jpg, *.tif, *.jpeg, *.tiff, *.bmp, and *.gif, and outputting
an HTML
# page to index those files. Links will create popup windows for the
individual images.
# The output html file will list the images in alphabetical order,
# given Perl's notion of alphabetical.

# Known limitations:
# Command line arguments are ignored for now. Sorry.
# Only works on current directory unless $dirname is modified prior to
runtime.
# Entire directory is read in at once and manipulated as an array. It's
possible
# that if the directory is truly massive, the array will overgrow memory
constraints.
# However, for this to happen, this little program is the least of your
worries; HELLO, try
# organizing your directory structure a little!

# The easiest way to customize this application is to modify the
declarations.

# --- Declarations ---

my ($outfile)="index.html"; # declare the output html file's name.
my (@imagefiles); # declare the list to hold the image files. Leave undef
until list retrieved.
my ($dirname) = "./"; # define the input directory.
my ($htmltitle)="Picture List"; # Give a title to the output html document
via <title> tag,
# and a first line <h1> caption.

# --- Main body ---

if (@imagefiles = getdir($dirname)) { #get list of image files or exit if
none found.
openhtmlfile($outfile); #open the output html file.
printheader($htmltitle); #print the html header.
printlinks(@imagefiles); #print the body links.
printfooter(); #print the terminating tags to the html file.
closehtmlfile(); #close the html output file.
} else {
print "Exiting: Current directory is empty.\n";
}

# --- End main body ---

# --- Subroutine definitions ---

sub getdir() { #Opens the current directory and gets all image filenames
into an array.
my ($dir)=@_;
my @entries;
opendir (CURRENTDIR, $dir) || die "Error: Unable to open directory: $dir \n
$!";
while (defined ( $file = readdir(CURRENTDIR))) {
if ($file =~ /.*\.((jpe?g)|(tiff?)|(bmp)|(gif))$/i) {
push (@entries,$file);
}
}
closedir(CURRENTDIR);
return sort(@entries);
}

sub openhtmlfile() { #Opens (@_) as file HTML for output.
my ($filename)=@_; #name the file to be opened using the input parameter.
if (-e $filename) { #Check. If file already exist prompt to overwrite or
exit.
print "Warning: $filename already exists. Overwrite? (y/n) ";
unless (<STDIN> =~ /^y/i) {
exit;
}
}
open (HTML, ">$filename") || die "Error: Unable to open $filename for
output.\n $!";
return;
}

sub printheader() { #Prints the html header to the outfile.
my ($title)=@_; #$title becomes both the html title as well as the <h1>
header.
print HTML "<html>\n<head>\n<title>$title</title>\n</head>\n";
print HTML "<body>\n";
print HTML "<h1>$title</h1>\n";
print HTML "<hr>\n<p>\n";
print HTML "Click on any link to open image in its own window. \n";
print HTML "By opening an image in its own window, you can print or easily
save ";
print HTML "individual images.\n";
print HTML "</p><hr>";
return;
}

sub printlinks() { #Prints an img src and href tag for each image in the
current dir.
my @filelist = @_;
foreach (@filelist) {
print HTML "<p>\n<img src=$_ alt=$_>\n<a href=\"$_\"
target=\"ImageWindow\"><br>$_</a>\n</p>\n";
}
return;
}

sub printfooter() { #Prints an html footer; the "closing" stuff.
print HTML "</body>\n</html>\n";
return;
}

sub closehtmlfile() { #Closes the html outfile.
close(HTML) || die "Error: Unable to close $filename after output.\n $!";
return;
}

--------------------------------------------

--
DJO

David Oswald · David Oswald

Thanks for the comments. I would like to hear more.
Yes, it was my first perl project. I read Learning Perl, Programming Perl,
Mastering Regular Expressions, about four years ago, cover to cover because
it intrigued me, but never tinkered until now when I finally found myself
with a little time. I had to brush up with a half-day of reading again
before I could dive in and create this. My other experience was a few
classes in Comp Sci about twelve years ago where I learned to tinker in
Pascal, C, and of all things, Modula II. I haven't touched C since about
1992 though, and the others disappeared from my vocabulary even before that.

Anyway, Perl intrigues me because it's one language where a lot can be
accomplished without getting bogged down in the details of user interfaces
and other such clutter. It just gets the job done quickly. Seems kind of
like putting together a puzzle.

In the following remarks I'm going to ask a few questions about your
comments:

"Janek Schleicher" <bigj@kamelfreund.de> wrote in message
news:pan.2003.06.26.04.25.11.180243@kamelfreund.de ...

> David Oswald wrote at Wed, 25 Jun 2003 23:35:55 -0700:
>
>

<snip>

> > my ($outfile)="index.html"; # declare the output html file's name.

>
> You can also leave out the brackets:
>
> my $outfile = "index.html";
>
> I would recommend it also, as the other tells Perl something very
> different as you primarly wanted (allthough it still works).
>

You're right. I should have caught that myself. For some reason after
brushing up I was still vague on that but I understand now.

> > # --- Subroutine definitions ---
> >
> >
> > sub getdir() { #Opens the current directory and gets all image

filenames

> > into an array.
> > my ($dir)=@_;
> > my @entries;
> > opendir (CURRENTDIR, $dir) || die "Error: Unable to open directory:

$dir \n

> > $!";
> > while (defined ( $file = readdir(CURRENTDIR))) {

>
> Where does $file come from?

I should have declared $file as "my $file;" within the subroutine getdir().
But without declaring it, its creation is implicit, though to your earlier
point, use strict; would balk at that notion, forcing me to declare it as I
should.

As far as how it gets loaded, readdir(CURRENTDIR) is loading it. As I'm
looking at it now, I'm wondering if I could leave out all references to
$file and thus imply $_? In other words, "while defined
readdir(CURRENTDIR)" .... would that load $_ with a value if I didn't
explicitly state $file= ? And then I could do away with the =~ operator in
my reg exp, and I would just push @entries,$_ ...just a thought.

> > if ($file =~ /.*\.((jpe?g)|(tiff?)|(bmp)|(gif))$/i) {

> ^^
>
> That's not necessary.
> A test whether a file has a specific ending is easier with:
>
> if ($file =~ /\.(jpe?g|tiff?|bmp|gif)$/i) {
>
> Note, that the many extra brackets you wrote,
> aren't necessary at all.
>

I should have looked up order of precidence again. But I was concerned
that, for example, the | (alternate) operator would grab "g" and "t" instead
of "jpe?g" and "bmp"... and so on. I wasn't sure how greedy | is. Why
isn't .* necessary if I'm trying to match the entire filename? I guess I
understand that; the entire filename gets loaded into $file even if I'm only
matching a portion of it.

> > push (@entries,$file);

Is there a better way of doing this? I read somewhere that it's best to
avoid subscript access, so push seemed like a good alternative. But is
there a construct that avoids any call at all? ...something like @entries=
(@entries,$file); ??

<snip>

>

> > sub printheader() { #Prints the html header to the outfile.
> > my ($title)=@_; #$title becomes both the html title as well as the

<h1>

> > header.
> > print HTML "<html>\n<head>\n<title>$title</title>\n</head>\n";
> > print HTML "<body>\n";
> > print HTML "<h1>$title</h1>\n";
> > print HTML "<hr>\n<p>\n";
> > print HTML "Click on any link to open image in its own window. \n";
> > print HTML "By opening an image in its own window, you can print or

easily

> > save ";
> > print HTML "individual images.\n";
> > print HTML "</p><hr>";

>
> Perhaps it's easier to use a HERE doc:
>
> print HTML <<END_OF_HTML;
> <html>
> <head>
> <title>$title</title>
> </head>
> <body>
> <h1>$title</h1>
> <hr>
> <p>
> Click on any link to open image in its own window.
> By opening an image in its own window, you can print or easily save
> individual images.
> </p>
> <hr>
> END_OF_HTML
>

Wow, I forgot all about here docs. The books only devote a couple
paragraphs to here documents, but you're right, that's a cleaner solution.

> You still play a lot with global variables,
> but the code is good structured and easy to understand.
> (If it's really your first Perl project, I'm impressed).

I think I had four globals. Did I miss something, or what could I have done
to further reduce that while maintaining re-configurability?

Thanks for taking the time to respond. I'd like to improve my proficiency.

Dave

>
>
> Greetings,
> Janek

Janek Schleicher · Janek Schleicher

David Oswald wrote at Wed, 25 Jun 2003 23:35:55 -0700:

In my first posting,
I forgot something important:

> sub getdir() { #Opens the current directory and gets all image

^^

> sub openhtmlfile() { #Opens (@_) as file HTML for output.

^^

> sub printheader() { #Prints the html header to the outfile.

^^

> sub printlinks() { #Prints an img src and href tag for each image in

^^

> sub printfooter() { #Prints an html footer; the "closing" stuff.

^^

> sub closehtmlfile() { #Closes the html outfile.

^^

You don't need in Perl the brackets to indicate that a sub declaration
follows.

In fact, it's very dangerous to use them unless you really want,
as Perl understands them as prototype declarations.
Prototyping gives a chance to change the way how Perl will parse the
calling of the subroutines (so you can probably leave out some more
brackets or referenciations), but that's a difficult job (for explaining
and doing) and the best recommandation I can give is not to use it.
If you want to know the details, read
perldoc perlsub

Greetings,
Janek

Janek Schleicher · Janek Schleicher

David Oswald wrote at Thu, 26 Jun 2003 02:41:20 -0700:

>> > while (defined ( $file = readdir(CURRENTDIR))) {

>>
>> Where does $file come from?

>
> I should have declared $file as "my $file;" within the subroutine getdir().
> But without declaring it, its creation is implicit, though to your earlier
> point, use strict; would balk at that notion, forcing me to declare it as I
> should.

Exactly. I only wanted to warn at that point, as I have seen (and done)
lot of mistakes, where I changed something on a different part of the
script and got confused that it failed now.

The main problem is not the missing declaration as declaration, but
without a 'my', the variable is _global_, and in larger scripts it's usual
to have somewhere else also another variable called 'file', thus there
could be very nasty side effects.

> As far as how it gets loaded, readdir(CURRENTDIR) is loading it. As I'm
> looking at it now, I'm wondering if I could leave out all references to
> $file and thus imply $_? In other words, "while defined
> readdir(CURRENTDIR)" .... would that load $_ with a value if I didn't
> explicitly state $file= ? And then I could do away with the =~ operator in
> my reg exp, and I would just push @entries,$_ ...just a thought.

That would do it.
You also could leave out something like
$_ =~ /.../
and
push @entries,$_
as the short versions
/.../
and
push @entries;
would also work

>> > if ($file =~ /.*\.((jpe?g)|(tiff?)|(bmp)|(gif))$/i) {

>> ^^
>>
>> That's not necessary.
>> A test whether a file has a specific ending is easier with:
>>
>> if ($file =~ /\.(jpe?g|tiff?|bmp|gif)$/i) {
>>
>> Note, that the many extra brackets you wrote,
>> aren't necessary at all.
>>

> I should have looked up order of precidence again. But I was concerned
> that, for example, the | (alternate) operator would grab "g" and "t" instead
> of "jpe?g" and "bmp"... and so on. I wasn't sure how greedy | is. Why
> isn't .* necessary if I'm trying to match the entire filename? I guess I
> understand that; the entire filename gets loaded into $file even if I'm only
> matching a portion of it.

The variable a regexp is patterned again is never changed!
You're right that with /.*.../ a successful match (contained in the
variable $& - read perldoc perlvar about) matches than the whole variable,
while a /.../ match only the ending in your case.
But that distinction doesn't play any role in your case, as you are not
interested in the matched stuff, only whether it matches the ending or not.

>> > push (@entries,$file);

>
> Is there a better way of doing this? I read somewhere that it's best to
> avoid subscript access, so push seemed like a good alternative. But is
> there a construct that avoids any call at all? ...something like @entries=
> (@entries,$file); ??

Sorry, I forgot to write the real Perlish way:

my @entries = grep /\.(jpe?g|tiff?|bmp|gif)$/i, readdir(CURRENTDIR);

instead of the whole loop.

>> You still play a lot with global variables,
>> but the code is good structured and easy to understand.
>> (If it's really your first Perl project, I'm impressed).

>
> I think I had four globals. Did I miss something, or what could I have done
> to further reduce that while maintaining re-configurability?
>

Five globals including $file :-))

Your globals had been all file handles like HTML or CURRENTDIR.
If you want to avoid them, you'll have to work with references to them.
Read e.g.
perldoc -q filehandle
about them.

I didn't explained it more detailed as it doesn't play a big role in
little scripts where I usually also work with global file handles.

Another way to avoid them would be to make a more strict clarification of
the jobs the subs would have to do.
Instead of e.g.
sub printheader {
...
print HTML ...;
}

you can always instead write a
sub header {
...
return $header;
}

and call it from the main program instead as printheader();
as
print HTML header();

Greetings,
Janek

Uri Guttman · Uri Guttman

repost with line wrapping disabled.

use a deeper indent. 1 char indent is ridiculous. 4 is a good choice

don't comment on the source lines. comment before them. that is hard to
read (especially with the wrap problem

more after you clean those things up

uri

--
Uri Guttman ------ [email]uri@stemsystems.com[/email] -------- [url]http://www.stemsystems.com[/url]
--Perl Consulting, Stem Development, Systems Architecture, Design and Coding-
Search or Offer Perl Jobs ---------------------------- [url]http://jobs.perl.org[/url]

Tad McClellan · Tad McClellan

David Oswald <spamblock@junkmail.com> wrote:

> Please offer suggestions, comments, and tips on the following perl code.

> I
> "overwrote" it to make it easy to modify in the future.

> # --- Declarations ---
>
> my ($outfile)="index.html"; # declare the output html file's name.

You should limit the scope of variables as much as possible, so
consider declaring them near their first use instead of way up
here at the top of the program.

Put spaces around operators, as suggested in:

perldoc perlstyle

my $outfile = "index.html";

Over-commenting *increases* maintenance time, since you say the
same thing twice, once in code and once in a comment.

my() already means "declare"

$outfile already implies what it is, due to the well-chosen variable name

> openhtmlfile($outfile); #open the output html file.

Thatsureisahardtoreadchoiceofname:

open_html_file($outfile);

> sub getdir() { #Opens the current directory and gets all image filenames
> into an array.

But it does not return that array.

It returns a list (as all subroutines do).

See this Perl FAQ:

What is the difference between a list and an array?

> while (defined ( $file = readdir(CURRENTDIR))) {
> if ($file =~ /.*\.((jpe?g)|(tiff?)|(bmp)|(gif))$/i) {
> push (@entries,$file);
> }
> }

You can use grep() to replace that whole while loop (untested):

my @entries =
grep { /.*\.((jpe?g)|(tiff?)|(bmp)|(gif))$/i } readdir CURRENTDIR;

> return sort(@entries);

Returns a "list", not an "array".

--
Tad McClellan SGML consulting
[email]tadmc@augustmail.com[/email] Perl programming
Fort Worth, Texas

David Oswald · David Oswald

Thanks, all your comments make sense.

Dave

"Tad McClellan" <tadmc@augustmail.com> wrote in message
news:slrnbfm2m2.6sf.tadmc@magna.augustmail.com...

<snipped for brevity>

David Oswald · David Oswald

I cleaned it up and perlized it considerably, and used here docs. But
somehow I broke it and can't figure out how to fix it. I apologize for the
minimal tabs. Something about how outlook interprets the tabs that exist in
the source. They're more generous irl. Here's what I've created:

_________________________

#!/usr/bin/perl -w

#imgdir2html.pl v1.02 (c)David Oswald

# The purpose of this script is to parse a single directory, read in all
# *.jpg, *.tif, *.jpeg, *.tiff, *.bmp, and *.gif filename, and output an
HTML
# page to index those files. Links will create popup windows for each
image.
# The output html file will list the images in alpha/perl order.

# Known limitations:
# Command line arguments are ignored for now.
# Only works in current directory unless $dirname is modified prior to
runtime.
# Entire directory is read in at once and manipulated as a list. Memory-
# sensitive code would read and process one directory item at a time.

# The simple way to customize this script is to modify the declarations.

use strict;

# --- Declarations ---

my $outfile ="index.html"; # Html file's name.
my @imagefiles;
my $dirname = "./"; # Input directory.
my $htmltitle ="Picture List"; # Html <title> and
# first line <h1> caption.
my $scriptver = "imgdir2html.pl v1.03";

# --- Main body ---

if (@imagefiles = getdir($dirname)) {
open_html_file($outfile);
print_head($htmltitle); #print the html header.
print_links(@imagefiles); #print the body links.
print_foot; #print the terminating tags.
close_html_file;
} else {
print "Exiting: Current directory is empty.\n";
}

# --- End main body ---

# --- Subroutine definitions ---

#Open the current directory and get all image filenames into a list.
sub getdir {
my ($dir)=@_;
opendir (CURRENTDIR, $dir)
|| die "Error: Unable to open directory: $dir \n $!";
my @entries = grep /\.(jpe?g|tiff?|bmp|gif)$/i, readdir(CURRENTDIR);
closedir(CURRENTDIR);
return sort(@entries);
}

#Opens (@_) as file HTML for output.
sub open_html_file {
my ($filename) = @_;
if (-e $filename) {
print "Warning: $filename already exists. Overwrite? (y/n) ";
unless (<STDIN> =~ /^y/i) {
exit;
}
}
open (HTML, ">$filename") || die "Error: Unable to open $filename for
output.\n $!";
return;
}

#Prints the html header to outfile.
sub print_head {
my ($title) = @_; #$title becomes <title> and <h1> headline.
print HTML <<HTML_BLOCK;
<html>
<head>
<title>$title</title>
</head>
<body>
<h1>$title</h1>
<hr>
<p>
Click on any link to open image in a separate window.
By opening an image in its own window, you can print
or easily save individual images.
</p>
<hr>
HTML_BLOCK
return;
}

#Prints <img src> and <a href> tag for each image.
sub print_links {
my @filelist = @_;
foreach (@filelist) {
print HTML <<HTML_BLOCK;
<p>
<img src = \"$_\" alt=$_ >
<a href = \"$_\" target = \"ImageWindow\" >
<br> $_ </a>
</p>
HTML_BLOCK
}
return;
}

#Prints an html footer; the "closing" stuff.
sub print_foot {
print HTML <<HTML_BLOCK;
<hr>
Page created by $scriptver .
<br>
</body>
</html>
HTML_BLOCK
return;
}

sub close_html_file { #Closes the html outfile.
close(HTML) || die "Error: Unable to close $filename after output.\n $!";
return;
}

-------------------------

And here's what errors I get when I now try to run it:

> imgdir2html.pl

Bareword "print_foot" not allowed while "strict subs" in use at
/home/pacifier..
Global symbol "$filename" requires explicit package name at
/home/pacifier.com/.
Bareword "close_html_file" not allowed while "strict subs" in use at
/home/paci.
Execution of /home/pacifier.com/d/da/davido.pacifier.com/pl/imgdir2html.pl
abor.

I think that the issue is that my here documents are somehow not being
terminated properly, resulting in subs not being defined. ...could that be?

Dave

----- Original Message -----
From: "Janek Schleicher" <bigj@kamelfreund.de>
Newsgroups: comp.lang.perl.misc
Sent: Thursday, June 26, 2003 4:09 AM
Subject: Re: Offer tips, comments on this code (generates html to index
image files)

David Oswald · David Oswald

Thanks to all for the input. Now that I've got it working again, and now
that it passes both "-w" and "use strict;", I'd like to hear comments again
from a style, efficiency, and lexicon standpoint: How can the existing code
be improved upon retaining equal functionality? In other words, I'm not
looking for how to add features. I'm looking at critiquing the script
holding its given featureset constant.

Here it is again:

______________________________________________

#!/usr/bin/perl -w

#imgdir2html.pl v1.03 (c)David Oswald

# The purpose of this script is to parse a single directory, read in all
# *.jpg, *.tif, *.jpeg, *.tiff, *.bmp, and *.gif filename, and output an
HTML
# page to index those files. Links will create popup windows for each
image.
# The output html file will list the images in alpha/perl order.

# Known limitations:
# Command line arguments are ignored for now.
# Only works in current directory unless $dirname is modified prior to
runtime.
# Entire directory is read in at once and manipulated as a list. Memory-
# sensitive code would read and process one directory item at a time.

# The simple way to customize this script is to modify the declarations.

use strict;

# --- Declarations ---

my $outfile ="index.html"; # Html file's name.
my @imagefiles;
my $dirname = "./"; # Input directory.
my $htmltitle ="Picture List"; # Html <title> and
# first line <h1> caption.
my $scriptver = "imgdir2html.pl v1.03";

# --- Main body ---

if (@imagefiles = getdir($dirname)) {
open_html_file($outfile);
print_head($htmltitle); #print the html header.
print_links(@imagefiles); #print the body links.
print_foot(); #print the terminating tags.
close_html_file();
} else {
print "Exiting: Current directory is empty.\n";
}

# --- End main body ---

# --- Subroutine definitions ---

#Open the current directory and get all image filenames into a list.
sub getdir {
my ($dir)=@_;
opendir (CURRENTDIR, $dir)
|| die "Error: Unable to open directory: $dir \n $!";
my @entries = grep /\.(jpe?g|tiff?|bmp|gif)$/i, readdir(CURRENTDIR);
closedir(CURRENTDIR);
return sort(@entries);
}

#Opens (@_) as file HTML for output.
sub open_html_file {
my ($ourfile) = @_;
if (-e $ourfile) {
print "Warning: $ourfile already exists. Overwrite? (y/n) ";
unless (<STDIN> =~ /^y/i) {
exit;
}
}
open (HTML, ">$ourfile") || die "Error: Unable to open $ourfile for
output.\n $!";
return;
}

#Prints the html header to outfile.
sub print_head {
my ($title) = @_; #$title becomes <title> and <h1> headline.
print HTML <<HTML_BLOCK;
<html>
<head>
<title>$title</title>
</head>
<body>
<h1>$title</h1>
<hr>
<p>
Click on any link to open image in a separate window.
By opening an image in its own window, you can print
or easily save individual images.
</p>
<hr>
HTML_BLOCK
return;
}

#Prints <img src> and <a href> tag for each image.
sub print_links {
my @filelist = @_;
foreach $_ (@filelist) {
print HTML <<HTML_BLOCK;
<p>
<img src = \"$_\" alt=$_ >
<a href = \"$_\" target = \"ImageWindow\" >
<br> $_ </a>
</p>
HTML_BLOCK
}
return;
}

#Prints an html footer; the "closing" stuff.
sub print_foot {
print HTML <<HTML_BLOCK;
<hr>
Page created by $scriptver .
<br>
</body>
</html>
HTML_BLOCK
return;
}

sub close_html_file { #Closes the html outfile.
close(HTML) || die "Error: Unable to close output file.\n $!";
return;
}

______________________

Again I apologize for the way Outlook interprets tabs as being one space
each. Duh. Anyway, the actual code IRL is properly tabbed.

Dave

A. Sinan Unur · A. Sinan Unur

"David Oswald" <spamblock@junkmail.com> wrote in
news:vfmqkhnh6eumf1@corp.supernews.com:

> Thanks to all for the input. Now that I've got it working again, and
> now that it passes both "-w" and "use strict;", I'd like to hear
> comments again from a style, efficiency, and lexicon standpoint: How
> can the existing code be improved upon retaining equal functionality?
> In other words, I'm not looking for how to add features. I'm looking
> at critiquing the script holding its given featureset constant.

I am not sure if this would be considered adding features, but when I was
working on a similar script, I found HTML::Template to be very useful. It
made it unnecessary to mix code with HTML generation.

If you would like to look at its usage, you can find the source code for my
script at [url]http://www.unur.com/comp/photobrowser/index.html[/url].

Sinan.

--
A. Sinan Unur
[email]asu1@c-o-r-n-e-l-l.edu[/email]
Remove dashes for address
Spam bait: mailto:uce@ftc.gov

Tad McClellan · Tad McClellan

David Oswald <spamblock@junkmail.com> wrote:

> somehow I broke it and can't figure out how to fix it.

> print HTML <<HTML_BLOCK;
> <p>
> <img src = \"$_\" alt=$_ >
> <a href = \"$_\" target = \"ImageWindow\" >
> <br> $_ </a>
> </p>
> HTML_BLOCK

> #Prints an html footer; the "closing" stuff.
> sub print_foot {

> And here's what errors I get when I now try to run it:

> Bareword "print_foot" not allowed while "strict subs" in use at

> I think that the issue is that my here documents are somehow not being
> terminated properly, resulting in subs not being defined. ...could that be?

Perhaps. But with a tool that mangles code between the real code and
the code that we can see, there's no way to tell.

Spaces matter a great deal to here-docs, we must have exactly
the right stuff if we are to spot potential problems.

It _looks_ OKish to me.

Here-docs are just an alternate form of quoting, switch them
one-by-one to some other form of quoting until the sub definitions
are found, eg:

print HTML "<H1>temp replacement for footer here</H1>\n";

--
Tad McClellan SGML consulting
[email]tadmc@augustmail.com[/email] Perl programming
Fort Worth, Texas

Tina Mueller · Tina Mueller

Janek Schleicher wrote:

> You also could leave out something like
> $_ =~ /.../
> and
> push @entries,$_
> as the short versions
> /.../
> and
> push @entries;
> would also work

no, push says: "Useless use of push with no values".
but it would be nice if push() worked like shift()
without arguments. i think i never tried that out before,
and now i was surprised that it didn't work =)
(would be even more perlish if you could say "push;"
instead of "push @_, $_;")

regards, tina
--
[url]http://www.tinita.de/[/url] \ enter__| |__the___ _ _ ___
[url]http://Movies.tinita.de/[/url] \ / _` / _ \/ _ \ '_(_-< of
[url]http://www.perlquotes.de/[/url] \ \ _,_\ __/\ __/_| /__/ perception
- my mail address expires end of august 2003 -

Eric Schwartz · Eric Schwartz

"David Oswald" <spamblock@junkmail.com> writes:

> Thanks to all for the input. Now that I've got it working again, and now
> that it passes both "-w" and "use strict;", I'd like to hear comments again
> from a style, efficiency, and lexicon standpoint: How can the existing code
> be improved upon retaining equal functionality? In other words, I'm not
> looking for how to add features. I'm looking at critiquing the script
> holding its given featureset constant.

First: prototype all your functions. That helps you immensely-- you'll know
instantly if you forgot a required argument.

'perldoc perlsub' to see how.

My personal style is to put the 'main' section at the end. Putting it
at the beginning, as you do, can come to bite you in the end (pun most
certainly intended). How, you ask? Simple: If someone adds a bit of
code between two sub definitions, it can get executed after your
"Finishing" message. This is bad, bad, bad! Putting your code at the
end makes it obvious where it ends, IMO.

> if (@imagefiles = getdir($dirname)) {

rename 'getdir' to 'get_images' or some such; a name like 'getdir'
suggests to me that it returns a directory name, or dirhandle.

> open_html_file($outfile);

You might consider having this return a filehandle, or an IO::File
object. You'd then pass that into the print_* functions and
close_html_file();

> #Open the current directory and get all image filenames into a list.
> sub getdir {

sub getdir($) {

See if you can come up with something that grabs you more than
'getdir'; I personally hate names like this as a coder, because I get
them stuck into my head, and can't ever think of something that
exactly captures the full nuances.... bleh. :)

> #Opens (@_) as file HTML for output.
> sub open_html_file {

sub open_html_file($) {

.... you get the idea. 'perldoc perlsub' for details.

> #Prints an html footer; the "closing" stuff.
> sub print_foot {
> print HTML <<HTML_BLOCK;
> <hr>
> Page created by $scriptver .

FYI, I know you said no features, but it'd be handy if it printed the
date the file was created as well-- I like knowing how old it's meant
to be.

-=Eric
--
Come to think of it, there are already a million monkeys on a million
typewriters, and Usenet is NOTHING like Shakespeare.
-- Blair Houghton.

David Oswald · David Oswald

"Eric Schwartz" <emschwar@pobox.com> wrote in message
news:etoadc4pia7.fsf@wormtongue.emschwar...
<snip>

> FYI, I know you said no features, but it'd be handy if it printed the
> date the file was created as well-- I like knowing how old it's meant
> to be.
>

That's a great idea. I'll get to work on it.
Having played with localtime(time); I'm not encouraged though. Surely there
must be a better way than trying to match a bunch of days of the week, month
names, and re-constructing 12hr time out of a 24hr format. Is there
something already lurking on the dusty shelf that converts the output of
localtime(time) to one nice $now of 12hr time so that I can print HTML "Last
updated: $now\n"; ?

Not to mention the fact that typing "date" at the shell prompt gives me a
correct date, yet the raw dump of "localtime(time)" thinks it's four in the
afternoon (it's ten at night really) December 31st, the 364th day of the
year (reality is June). I suspect that my sysadmin has the time wrong on
the computer that's hosting the perl engine.

Dave

Anno Siegel · Anno Siegel

Tina Mueller <usenet@expires082003.tinita.de> wrote in comp.lang.perl.misc:

> Janek Schleicher wrote:

> > You also could leave out something like
> > $_ =~ /.../
> > and
> > push @entries,$_
> > as the short versions
> > /.../
> > and
> > push @entries;
> > would also work

>
> no, push says: "Useless use of push with no values".
> but it would be nice if push() worked like shift()
> without arguments.

Ah, but push() and shift() don't correspond to each other, push() and
unshift() do. Unshift() behaves like push() in that it requires two
arguments. Of course, the second argument must be there only formally,
it is allowed to be an empty list.

> i think i never tried that out before,
> and now i was surprised that it didn't work =)
> (would be even more perlish if you could say "push;"
> instead of "push @_, $_;")

Perl doesn't use @_ as a default parameter nearly as much as it
uses $_. AFAIK only shift() and pop() do, plus one particular call
of split() which is deprecated. This is probably because $_ is really
a no-mans-variable while @_ is part of Perl's parameter passing
mechanism.

Anno

Bryan Castillo · Bryan Castillo

"David Oswald" <spamblock@junkmail.com> wrote in message news:<vfmqkhnh6eumf1@corp.supernews.com>...

> Thanks to all for the input. Now that I've got it working again, and now
> that it passes both "-w" and "use strict;", I'd like to hear comments again
> from a style, efficiency, and lexicon standpoint: How can the existing code
> be improved upon retaining equal functionality? In other words, I'm not
> looking for how to add features. I'm looking at critiquing the script
> holding its given featureset constant.
>

Im surprised no one else mentioned this.

I think it is good practice to pass references to filehandles to
subroutines, instead of using a global filehandle.

(Example)

open(HTML, ">out.html") || die;
call_sub1(\*HTML);

or

use IO::File;
my $html = IO::File->new(">out.html") || die;
call_sub1($html);

sub call_sub1 {
my $html = shift;
print $html "<h1>print something</h1>\n";
}

Eric Schwartz · Eric Schwartz

"David Oswald" <spamblock@junkmail.com> writes:

> Having played with localtime(time); I'm not encouraged though.

'perldoc POSIX'; search for 'strftime'. It is your friend.

> Not to mention the fact that typing "date" at the shell prompt gives me a
> correct date, yet the raw dump of "localtime(time)" thinks it's four in the
> afternoon (it's ten at night really) December 31st, the 364th day of the
> year (reality is June). I suspect that my sysadmin has the time wrong on
> the computer that's hosting the perl engine.

Most likely, it's running in UTC, which is fairly common for servers.
You can either convert to local time, or (my favourite) just write
"<time> UTC".

-=Eric
--
Come to think of it, there are already a million monkeys on a million
typewriters, and Usenet is NOTHING like Shakespeare.
-- Blair Houghton.

David Oswald · David Oswald

Will a reference to a filehandle eliminate the issue of having "HTML"
filehandle suddenly become global? I thought that it does so regardless.
From what I can surmise, passing the filehandle as a reference basically
allows the filehandle to be created under a different name, and after that
all other subs will just reference it as a "reference" thus eliminating the
need to hardwire the name itself into the other subs. Correct?

Dave

"Bryan Castillo" <rook_5150@yahoo.com> wrote in message
news:1bff1830.0306270749.47558552@posting.google.c om...

> "David Oswald" <spamblock@junkmail.com> wrote in message

news:<vfmqkhnh6eumf1@corp.supernews.com>...

> > Thanks to all for the input. Now that I've got it working again, and

now

> > that it passes both "-w" and "use strict;", I'd like to hear comments

again

> > from a style, efficiency, and lexicon standpoint: How can the existing

code

> > be improved upon retaining equal functionality? In other words, I'm not
> > looking for how to add features. I'm looking at critiquing the script
> > holding its given featureset constant.
> >

>
> Im surprised no one else mentioned this.
>
> I think it is good practice to pass references to filehandles to
> subroutines, instead of using a global filehandle.
>
> (Example)
>
> open(HTML, ">out.html") || die;
> call_sub1(\*HTML);
>
> or
>
> use IO::File;
> my $html = IO::File->new(">out.html") || die;
> call_sub1($html);
>
> sub call_sub1 {
> my $html = shift;
> print $html "<h1>print something</h1>\n";
> }

Offer tips, comments on this code (generates html to index image files)

Thread Tools

Display

Offer tips, comments on this code (generates html to index image files)

Similar Questions and Discussions

Help with creating a control that generates dynamic image

Offer a Choice of Flash or HTML?

regexp for parsing image filenames out of html code

How to color-code HTM and HTML files like ASP files?

asp:form generates invalid code

Re: Offer tips, comments on this code (generates html to index image files)

Re: Offer tips, comments on this code (generates html to index image files)

Re: Offer tips, comments on this code (generates html to index image files)

Re: Offer tips, comments on this code (generates html to indeximage files)

Re: Offer tips, comments on this code (generates html to index image files)

Re: Offer tips, comments on this code (generates html to index image files)

Re: Offer tips, comments on this code (generates html to index image files)

Re: Offer tips, comments on this code (generates html to index image files)

Re: Offer tips, comments on this code (generates html to index image files)

Re: Offer tips, comments on this code (generates html to index image files)

Re: Offer tips, comments on this code (generates html to index image files)

Re: Offer tips, comments on this code (generates html to index image files)

Re: Offer tips, comments on this code (generates html to index image files)

Re: Offer tips, comments on this code (generates html to index image files)

Re: Offer tips, comments on this code (generates html to index image files)

Re: Offer tips, comments on this code (generates html to index image files)

Re: Offer tips, comments on this code (generates html to index image files)

Posting Permissions