On Mon, Oct 21, 2002 at 04:32:27PM -0500, Akens, Anthony wrote:
[snip]

It looks like you forgot -w and use strict.


> open (NAMES, $namefile)
>     or print "Could not open $namefile $!";

Do you really want to continue and read the file if the open fails?

     
> while(<NAMES>)
> {
>     ($key, $value) = split /:/;

Is it possible to have colons in the display name?  If so you should limit
your split:

      ($key, $value) = split /:/, $_, 2;

>     chomp $value;
>     $Names{$key} = $value;
> }
> close NAMES;
> 
> %seen=();
> print "<P>";
> 
> foreach $dirname (sort { "\L$Names{$a}" cmp "\L$Names{$b}" } keys %Names){
>     $displayname = $Names{$dirname};
>     if (($displayname eq "MISSING") or ($displayname eq "HIDE")){}
>     else{

That empty block there looks odd.  The typical way to do this is:

      unless (($displayname eq "MISSING") or ($displayname eq "HIDE")) {

or
      if (($displayname ne "MISSING") and ($displayname ne "HIDE")) {


Though I would use:

      unless ($displayname eq 'MISSING' || $displayname eq 'HIDE') {

but only because I prefer '||' to 'or', and don't like doubling up on
parens if it can be avoided.


>         
>         $item = substr($displayname, 0, 1);
>         if ($seen{"\L$item"}++){}
>         else {

Here's another empty block; you should avoid these.  Use:

          unless ($seen{"\L$item"}++) {

or
          if (!$seen{"\L$item"}++) {

instead.


>             print "<A NAME=\"$item\">\U$item<\/a><br>\n";
>         }
>         print "<a href=\"installer.pl?dirname=$dirname&displayname=$displayname\">";

You can avoid having to escape quotes by using different quote delimiters:

          print qq{<a href="installer.pl...">};

If installer.pl is the name of the script this code is in you might be
better off using $ENV{SCRIPT_NAME}.


>         print $displayname;
>         print "<\/a><br>\n";

You don't need to escape forward slashes "/", just backslashes.

I'd suggest combining your print statements:

          print(
              qq{<a href="installer.pl...">},
              qq{$displayname</a><br>\n}
          );

>     }
> }

Personally, I would avoid the "\L..." and "\U..." in favor of function
calls, i.e. lc(...) and uc(...).  This is a personal preference arising from
the fact that it's easy for me to overlook the \L and \U in a string.

Everything else looks fine.


Michael
--
Administrator                      www.shoebox.net
Programmer, System Administrator   www.gallanttech.com
--

-- 
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to