Hi!

I got some thoughts on your script, I'd like to share with you. You dared to ask, so you deserve no better. HARHAR!! ;) Not all of it is CGI-related, but nonetheless I hope it's interesting.

my $sql_stmnt = "SELECT emp_id, CONCAT(emp_last, ', ', emp_first), dep
+t_name
                 FROM sys_main.humans, sys_main.depts 
                 WHERE status='AV' AND dept_id=dept
                 ORDER BY '$sort'";
my $sth = $dbh->prepare($sql_stmnt);
$sth->execute();
[download]

This is fine. I do not know mySQL (our DB folks like Oracle ;), but if it supports bind variables in the ORDER BY clause, you could say:

my $sth = $dbh->prepare_cached(<<"+++end_of_sql_statement");
  SELECT emp_id, CONCAT(emp_last, ', ', emp_first), dept_name
  FROM sys_main.humans, sys_main.depts 
  WHERE status='AV' 
    AND dept_id=dept
  ORDER BY ?
+++end_of_sql_statement

$sth->execute($sort);
[download]

Using prepare_cached might save you some milliseconds, if mySQL allows for it. If cached, using a placeholder in the ORDER BY clause will allow to have the statement cached independent from the sort column. Unless you need the value of $sort elsewhere you could even say:

to leave things in place. Also, having sort double quoted is a waste of time, at least of optimizer time, but that's pernickety. ;)

Just as a matter of taste, I like to put my SQL statments in here docs, as above.

$q->start_html(-title=>'Employee Table', -bgcolor=>'#FFFFFF', 
                  -link=>'#4682B4', -vlink=>'#5F9EA0',
                  -alink=>'#1E90FF')

$q->start_table({-width=>'450', -border=>'0',
                    -cellpadding=>'2', -cellspacing=>'0'})
[download]

Wouldn't you wan't have all those style attributes defined in the stylesheet you link to? Even if the imported stylesheet is not in your hands, you could define your styles in a dedicated style tag in the head section of the generated output:

my $stylesheet = <<'++end_of_stylesheet';
  .emp_table {
    width:        450 pt;
    border-width: 0 pt;
    padding:      2 pt;
  }
++end_of_stylesheet

print $q->start_html(-title => 'Employee Table', 
                     -style => { -code =>$stylesheet }),
                     
      $q->start_table({ -class => 'emp_table', 
                        -width => 450 });
[download]

Note that CGI.pm also provides a span() function allowing you to apply a class / style definition to multiple elements (which would save some of your HTML span tags). I am not sure, however, if that works in the OO interface, too.

You hard-code the (own) script name several times:

-href=>"emp-list.cgi"
[download]

Don't. Either use my $SCRIPT_NAME = 'emp_list.cgi' at file scope near the top of your file, so you'd have to change that only once if it changes. Even better is to use $q->script_name() for that. That way, you can rename the script file without having to change anything.

Use as few literals as possible. Early in your script use:

my $STYLESHEET_DIR = '/stylesheets';
my $IMAGE_DIR      = '/images';
[download]

Actually, I like have such definitions in a CONF.pm and use that. Running under mod_perl, this will even save a bit memory making these variables shared.

while (@_ = $sth->fetchrow_array()) {
   $emp_id = $_[0];
   $emp_name = $_[1];
   $dept = $_[2];
[download]

Consider:

while (my ($emp_id, $emp_name, $dept) = $sth->fetchrow_array) 
{ ... }
[download]

Hope this helps/is interesting,
so long
Flexx