RFC - I intend to make this node a tutorial in writing a good, clean cgi/sql application - any suggestions on how best to structure it?

My main concern is that full code-listings are going to swamp it, but on the other hand I want such code-listing available as the code is revised and improved - should I put the code in a seperate node?

Moving on from the RFC, the purpose of this node is elaborated on below...

* * * * * * *

I'm often asked at work to implement a quick database with a web front-end for some task or other...

... and to be frank, a lot of the code I write sucks.

Now I have been asked to write yet-another-bit-of-perl/sql code, but I thought it would be more informative for both myself and others if I rewrote some existing code first and documented the process on perlmonks.

This would give a proper before/after flavour. Needless to say, I'm hoping for some input from the clergy.

So, I'm going to take the following shortish bit of code, and rewrite using CGI::Application and HTML::Template.

I'll also place and test-version of this app and make that available as a resource to accompany this node.

Now (take a deep breath), here is a few samples of the code that I'm going to re-engineer (missing quite a few 'states' and subroutines, but hopefully showing off a few of its sins):

#!/usr/bin/perl
use strict;
use DBI;
use CGI qw(:standard);

my $database = 'cvkb';
my ($script_url, $css_url, $host, $server_type, $data_source, $login, 
+$password);
my %db_cols;
$db_cols{'all'} = 'oid,question,answer,author,keywords,sites,server,cl
+ient,application,fixed_in';
$db_cols{'list'} = 'oid,question';
$db_cols{'add'} = 'question,answer,author,keywords,sites,server,client
+,application,fixed_in';
$db_cols{'edit'} = 'question,answer,author,keywords,sites,server,clien
+t,application,fixed_in';

my %fld_type = (
  'answer'=>'textarea',
  'question'=>'textarea',
);

my $msg;

$login = 'foobar';
$password = 'barfoo';
$host = 'GONK1';
$server_type = 'my-sql';
$data_source = "DBI:mysql:database=$database;host=$host";
$script_url = './cvfaq.cgi';
$css_url = '/cvfaq/cvfaq.css';

my $error_file = './cvkb.err';
my $log_file = './cvkb.sql';
my $log_sql = 1;

### Main Program

open(ERR, ">>$error_file");
open(LOG, ">>$log_file") if $log_sql;

my $dbcon =
DBI->connect($data_source,$login,$password,{PrintError=>0,RaiseError=>
+0,AutoCommit=>1});
my $connect_error = DBI::errstr;
if($connect_error){
  $msg .= $connect_error . ' : ' . $data_source;
  kb_top();kb_content('Connect Error', 0);kb_tail();
  exit 0;
}

clean_input();
my $action = param('action');
CGI::delete('action');

if(!$action or $action eq 'list_kbs'){
  return_list_page();
  exit 0;
}
elsif($action eq 'add'){
  my @cols = split /,/, $db_cols{'add'};
  my $insert_vals;
  foreach(@cols){$insert_vals .= "'" . param($_) . "',";}
  chop $insert_vals;
  my $sql = "insert into kb ($db_cols{'add'}) values ($insert_vals)";
  my $qh = run_sql($sql);
  $sql = "select max(oid) from kb";
  $qh = run_sql($sql);
  my @data = $qh->fetchrow_array;
  return_kb_page($data[0]);
}
else{
  return_full_page('Unknown command: $action',0);
  exit 0;
}
exit 0;

#### Sub-Routines ####

sub run_sql{
  my $sql = $_[0];
  $dbcon->{LongReadLen}=5000;
  $dbcon->{LongTruncOk}=1;
  my $qh = $dbcon->prepare($sql);
  if($dbcon->errstr){
    $msg .= $dbcon->errstr . ' : ' . $sql;
    print ERR gettime() . $msg . "\n\n";
    return 0;
  }
  else{
    $qh->execute;
    if($qh->errstr){
      $msg .= $qh->errstr . ' : ' . $sql;
      print ERR gettime() . $msg . "\n\n";
      return 0;
    }
  }
  print LOG gettime() . ': ' . $sql . "\n" if $log_sql;
  return $qh;
}

sub return_kbs{
  my($sql, $qh, @data, @result);
  if(ref $_[1]){
    my $list = join ',', @{$_[1]};
    $sql = "select $_[0] from kb where oid in($list) order by oid";
  }
  elsif($_[1]){
    $sql = "select $_[0] from kb where oid = $_[1] order by oid";
  }
  else{
    $sql = "select $_[0] from kb order by oid";
  }
  $qh = run_sql($sql);
  if($qh){
    while(@data = $qh->fetchrow_array){
      push @result, [@data];
    }
    $qh->finish;
  }
  return @result;
}

sub return_kb_page{
  my @kb;
  my $content = '<table>';
  @kb = return_kbs($db_cols{'all'},$_[0]);
  my @labels = split /,/, $db_cols{'all'};
  my $count = 0;
  foreach(@{$kb[0]}){
    $_ = html_escape($_);
    if($fld_type{$labels[$count]} eq 'textarea'){
      s/^(.*)$/<p>$1<\/p>/s;
      s/^\s*$/<\/p><p>/gm;
    }
    $content .= "<tr><th>" . ucfirst $labels[$count] .
      "</th><td>$_</td></tr>\n";
    $count ++;
  }
  $content .= '<tr><td>' . start_form(-method=>'GET') . hidden('oid', 
+$_[0]) .
    hidden('action','edit_form') . submit(-value=>'Edit KB') .
    end_form() . '</td></tr>';
  $content .= '</table>';
  return_full_page('CV KB', $content);
}

[download]
Tom Melly, tom@tomandlu.co.uk

Replies are listed 'Best First'.
Re: RFC: CGI/MySQL - improving my code
by dragonchild (Archbishop) on Mar 27, 2006 at 11:37 UTC
    Using your code as a specification, rewrite it using CGI::Application, DBIx::Class (or Class::DBI), and Template Toolkit. While there's a learning curve on each of those distributions, you will thank yourself in the morning.
    My criteria for good software:
    1. Does it work?
    2. Can someone else come in, make a change, and be reasonably certain no bugs were introduced?
[reply]
      two hints if you decide to rewrite your app and if you want a good cgi:
      • use taint mode
      • add a apache mod_perl AND a cgi configuration, e.g. 127.0.0.1/perl/script.pl runs your script in mod_perl, 127.0.0.1/cgi-bin/script.pl in classical cgi environment.
      developing in cgi has some advantages (you see performance problems better, you don't have to restart apache) but you also should test mod_perl regulary.
[reply]

      Hmm, well CGI::Application is already on my list.

      I intend to use HTML::Template rather than Template Toolkit - any thoughts?

      I'll look into those DBI modules - I did wonder whether I should just carry on implementing DBI or whether there were useful modules that present a cleaner code-base for sql transactions (my reservations about such a module would be that it either makes too many assumptions about what you want to do, or ends up not really being much different from rolling your own sql).

      BTW fixed typo - HTML::Template rather than HTML::Application iirc

      Tom Melly, tom@tomandlu.co.uk
[reply]
        I agree with dragonchild: Template Toolkit is very much worth the learning curve.

        I've been using HTML::Template for a long time now, but TT's dot notation alone is such an amazing feature that it's inspired Mark Stosberg and me to write HTML::Template::Pluggable to provide HTML::Template with the same feature. You can take it from me that TT's implementation is far superior.

[reply]
        Even if you write the database objects yourself from scratch, it's still worth doing, to isolate that code from the rest of your program. You may actually want to try writing it from scratch once, just to see what's involved.

        In most cases you can mix your own hand-written SQL with these modules. You'll find that Rose::DB::Object provides the ability to do almost any SQL you need without resorting to that though.

[reply]
      Or Catalyst.

      CountZero

      "If you have four groups working on a compiler, you'll get a 4-pass compiler." - Conway's Law

[reply]
Re: RFC: CGI/MySQL - improving my code
by perrin (Chancellor) on Mar 27, 2006 at 18:07 UTC
    A few basics:
    • Use a config file. This database login stuff does not belong in your application code.
    • Don't write your own HTML escaping code. Use CGI or HTML::Entities.
    • Use templating to get the HTML out of your code.
    • Use CGI::Application or something similar to get rid of that if/elsif tree for $action.
    • Turn RaiseError on for DBI.
    Also, if you want detailed responses, post less code. It would take hours to go through this code carefully.
[reply]

      To be honest, I didn't expect anyone to go through my code - I'll revise my text to indicate that - it was more meant as an example of how a combination of lack of knowledge and planning can lead to messy code.

      What I'd really like to do is have an orphaned node that I can just link to (the code ought to be available, but doesn't need to be in the main body of the article).

      I'd really like to create a more structured article than (afaik) perlmonks allows - e.g. comments only accessable if you request them.

      Tom Melly, tom@tomandlu.co.uk
[reply]
Re: RFC: CGI/MySQL - improving my code
by submersible_toaster (Chaplain) on Mar 28, 2006 at 10:06 UTC

    Seriously seriously consider CountZero's brief but pointed advice. Catalyst can make this sort of thing not just easy - but also <gulp> fun?!. I have toyed about with CGI::Application and it's magical variants - Catalyst works and works Nice! Get to grips with Class::DBI or DBIx::Class and you can write these sort of throwaways in your sleep. Catalyst helpers will write half of it for you.

    I should explain that whilst on the mailing list and using catalyst on several of my projects , I am not affiliated or related to the Catalyst team.


    I can't believe it's not psellchecked
[reply]
Re: RFC: CGI/MySQL - improving my code
by radiantmatrix (Parson) on Mar 28, 2006 at 18:41 UTC
[reply]

      Ovid's CGI course doesn't even address templating and seems to me to be about 5-10 years out of date wrt good programming practice.

      UPDATE: Ovid's course is great for what it is; I'm merely pointing out that it is hardly the equivalent of what the OP is proposing.

[reply]

Back to Meditations