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:

$sth->execute($q->param('sort') || 'emp_last')

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

In reply to Re: A Matter of Style in CGI by Flexx
in thread A Matter of Style in CGI by Spenser

Title:
Use:  <p> text here (a paragraph) </p>
and:  <code> code here </code>
to format your post, it's "PerlMonks-approved HTML":


  • Posts are HTML formatted. Put <p> </p> tags around your paragraphs. Put <code> </code> tags around your code and data!
  • Titles consisting of a single word are discouraged, and in most cases are disallowed outright.
  • Read Where should I post X? if you're not absolutely sure you're posting in the right place.
  • Please read these before you post! —
  • Posts may use any of the Perl Monks Approved HTML tags:
    a, abbr, b, big, blockquote, br, caption, center, col, colgroup, dd, del, details, div, dl, dt, em, font, h1, h2, h3, h4, h5, h6, hr, i, ins, li, ol, p, pre, readmore, small, span, spoiler, strike, strong, sub, summary, sup, table, tbody, td, tfoot, th, thead, tr, tt, u, ul, wbr
  • You may need to use entities for some characters, as follows. (Exception: Within code tags, you can put the characters literally.)
            For:     Use:
    & &amp;
    < &lt;
    > &gt;
    [ &#91;
    ] &#93;
  • Link using PerlMonks shortcuts! What shortcuts can I use for linking?
  • See Writeup Formatting Tips and other pages linked from there for more info.