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.
- Hard to read
- Hard to modify
- Poor scalability (or at least, scalability not considered)
- Bad practices - e.g. no proper locking
- Just really, really ugly
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,client,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,client,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 = '| " . ucfirst $labels[$count] . " | $_ |
|---|---|
| ' . start_form(-method=>'GET') . hidden('oid', $_[0]) . hidden('action','edit_form') . submit(-value=>'Edit KB') . end_form() . ' |
My criteria for good software:
- Does it work?
- Can someone else come in, make a change, and be reasonably certain no bugs were introduced?
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
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.
In most cases you can mix your own hand-written SQL with these modules. You'll find that CPAN://Rose::DB::Object provides the ability to do almost any SQL you need without resorting to that though.
- 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.
- 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.
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.
Seriously seriously consider CountZero's brief but pointed advice. Catalyst can make this sort of thing not just easy - but also
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
I admire your willingness to write a CGI/SQL tutorial, but Ovid's CGI course already exists, and is very good. Perhaps you could just summarize the material there instead of re-doing the work?
A collection of thoughts and links from the minds of geeks
The Code that can be seen is not the true Code
I haven't found a problem yet that can't be solved by a well-placed trebuchet