You are viewing a plain text version of this content. The canonical link for it is here.
Posted to asp@perl.apache.org by Philip Mak <pm...@aaanime.net> on 2002/01/08 10:38:58 UTC

[OT] Good coding practice

I was just looking over an old piece of code that I wrote for
AnimeLyrics.com. It was part of an ASP script used to process a form.
It was very ugly:

  my $lnum = '';
  my $snum = $Request->Form('snum');
  my $action = $Request->Form('action');
  my $lhandle = $Request->Form('lhandle');
  my $collection = $Request->Form('collection');
  my $track = $Request->Form('track');
  my $title = $Request->Form('title');
  my $etitle = $Request->Form('etitle');
  my $description = $Request->Form('description');
  my $note = $Request->Form('note');
  my $footnote = $Request->Form('footnote');
  my $romaji = $Request->Form('romaji');
  my $english = $Request->Form('english');
  my $kanji = $Request->Form('kanji');
  my $contributor = $Request->Form('contributor');
  my $legacy = $Request->Form('legacy');
  my $contributor2 = $Request->Form('contributor2');
  my $jl_type = $Request->Form('jl_type');
  my $jl_shandle = $Request->Form('jl_shandle');
  my $jl_lhandle = $Request->Form('jl_lhandle');

  my $db_snum = $dbh->quote($snum);
  my $db_lhandle = $dbh->quote($lhandle);
  my $db_collection = $collection ? $dbh->quote($collection) : 'NULL';
  my $db_track = $track ? $dbh->quote($track) : 'NULL';
  my $db_title = $dbh->quote($title);
  my $db_etitle = $dbh->quote($etitle);
  my $db_description = $dbh->quote($description);
  my $db_note = $dbh->quote($note);
  my $db_footnote = $dbh->quote($footnote);
  my $db_romaji = $dbh->quote($romaji);
  my $db_english = $dbh->quote($english);
  my $db_kanji = $dbh->quote($kanji);
  my $db_legacy = $dbh->quote($legacy);
  my $db_jl_type = $dbh->quote($jl_type);
  my $db_jl_shandle = $dbh->quote($jl_shandle);
  my $db_jl_lhandle = $dbh->quote($jl_lhandle);

Actually, it wasn't so bad in the beginning when that form only had a
few fields. But as AnimeLyrics.com evolved, I found the need to add
more fields in... and I ended up with the monster that you see now.

If I had to write it again, I would do something like this instead:

my %f = %{$Request->Form};
my %d = ();
for (keys %f) {
  $d{$_} = $dbh->quote($f{$_});
}
$d{collection} = 'NULL' unless $f{collection};
$d{track} = 'NULL' unless $f{track};

and then I can access all the form variables using %f, and the
database quoted version of the form variables using %d.

---------------------------------------------------------------------
To unsubscribe, e-mail: asp-unsubscribe@perl.apache.org
For additional commands, e-mail: asp-help@perl.apache.org


Re: [OT] Good coding practice

Posted by Joshua Chamas <jo...@chamas.com>.
Philip Mak wrote:
> ...
> If I had to write it again, I would do something like this instead:
> 
> my %f = %{$Request->Form};
> my %d = ();
> for (keys %f) {
>   $d{$_} = $dbh->quote($f{$_});
> }
> $d{collection} = 'NULL' unless $f{collection};
> $d{track} = 'NULL' unless $f{track};
> 
> and then I can access all the form variables using %f, and the
> database quoted version of the form variables using %d.
> 

This is a good tip on better coding practice... futher 
I would recommend that people not use $dbh->quote() at all,
but rather make use of bind parameters like:

  my $form = $Request->Form();
  my $sth = $dbh->prepare("select * from sometable where column1 = ?");
  $sth->execute($form->{column1});
  my $row = $sth->fetchrow_array;
      - or even simpler -
  my $form = $Request->Form();
  my $row = $dbh->selectrow_array("select * from sometable where column1 = ?", undef, $form->{column1});

I know in perl there are many ways, but using bind parameters let you 
never to have to use $dbh->quote ( I have never had to use it ), 
or default to NULL ( since undef is NULL for bind params ), with the 
added benefit for some databases that cache execution plans like Oracle, 
using bind paramters will let the SGA cache statements that are prepared 
previously better, without bind parameters any kind of db statement plan 
cache would be flushed pretty quickly.

--Josh
_________________________________________________________________
Joshua Chamas                           Chamas Enterprises Inc.
NodeWorks Founder                       Huntington Beach, CA  USA 
http://www.nodeworks.com                1-714-625-4051

---------------------------------------------------------------------
To unsubscribe, e-mail: asp-unsubscribe@perl.apache.org
For additional commands, e-mail: asp-help@perl.apache.org