Loading ...
Sorry, an error occurred while loading the content.

[OT] Good coding practice

Expand Messages
  • Philip Mak
    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
    Message 1 of 2 , Jan 8, 2002
    • 0 Attachment
      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@...
      For additional commands, e-mail: asp-help@...
    • Joshua Chamas
      ... 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
      Message 2 of 2 , Jan 8, 2002
      • 0 Attachment
        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@...
        For additional commands, e-mail: asp-help@...
      Your message has been successfully submitted and would be delivered to recipients shortly.