Stories
Slash Boxes
Comments
NOTE: use Perl; is on undef hiatus. You can read content, but you can't post it. More info will be forthcoming forthcomingly.

All the Perl that's Practical to Extract and Report

use Perl Log In

Log In

[ Create a new account ]

heusserm (4461)

heusserm
  (email not shown publicly)
http://www.xndev.com/
AOL IM: MatthewHeusser (Add Buddy, Send Message)

Matt Heusser is JAPH and an XP-Er [xprogramming.com]. (The Methodology, not the Operating System.) Right now, he's doing a lot of Test Driven Development in Perl.

Journal of heusserm (4461)

Wednesday September 17, 2003
06:33 AM

Exception Handling ... hmm

[ #14748 ]
Right now, I'm running into a lot of legacy code like this:

#------------------------#
sub GetValues
#------------------------#
{
    my $dbh = get_connection('databasename');
    my $sth = $dbh->prepare('Select * from table_name');
    $sth->execute();
    my $r;
    my @a;
    while ($r = $sth->fetchrow_hashref) {
        push @a, $r;
    }
    return (\@a);
}

my "Improved" version would look more like this:

#------------------------#
sub GetValues
#------------------------#
{
    my $dbh = get_connection('databasename');
    my $ok = 1;
    my $msg = "Success";
    my $sth;
    if (!$dbh) {
        $ok = 0;
        $msg = "Failed to get connection to databasename in GetValues";
    } else {
        $sth = $dbh->prepare('Select * from table_name');
    }

    if ($ok && (!$sth)) {
        $ok = 0;
        $msg = "Call to prepare failed in GetValues";
    }

    my $r;
    my @a;
    if ($ok) {
       $ok = $sth->execute();
       if (!$ok) {
           $msg = "Call to excecute failed in GetValues";
       } else {
           my $r;
           my @a;
           while ($r = $sth->fetchrow_hashref) {
               push @a, $r;
       }
    }
    return ($ok, $msg, \@a);
}
#------------------------#

  Note that my new code is literally TWICE as long as the old code.  Often, it's longer.

  Is this really advantageous?  In the long run, it feels like I'm trying the same things over and over again.  I actually made a module to abstract out the if-ish ness of the this routine, but the functions got to the point where they were taking 6+ paramters, some of which were optional.  It was just ugly.  So I've stopped using it until I can do it over again with anonymous hashrefs and "named params."

  The net result is that plenty of folks are saying "error checking is over-rated", and I can see where they are coming from.  What's the "right" answer?

The Fine Print: The following comments are owned by whoever posted them. We are not responsible for them in any way.
 Full
 Abbreviated
 Hidden
More | Login | Reply
Loading... please wait.
  • If the get_connection method returns a handle
    on which RaiseError, was set, just wrap in
    an eval:

    sub GetValues {
          my $a;

          eval {
                  my $dbh = get_connection('databasename');
                  my $sth = $dbh->prepare('Select * from table_name');
                  $sth->execute();
                  my $r;
         
    --
    -derby
  • bail out on error (Score:3, Interesting)

    by jmm (276) <johnNO@SPAMperlwolf.com> on 2003.09.17 11:03 (#24270) Journal
    A lot of the extra code space is the constant testing whether you need to skip the next step because you've already hit an error. Just bail out when the error happens. I also changed it to either return 0, $msg (when an error happened) or else to return 1,@list (when everything worked).
    #------------------------#
    sub GetValues
    #------------------------#
    {
        my $dbh = get_connection('databasename')
          or return( 0, "Failed to get connection to databasename in GetValues" );
        my $sth = $dbh->prepare('Select * from table_name')
          or return( 0, "Call to prepare failed in GetValues" );
        my @a = $sth->execute()
          or return( 0, "Call to excecute failed in GetValues" );
        my $r;
        my @a;
        push @a, $r
            while $r = $sth->fetchrow_hashref;
        return (1, \@a);
    }
    #------------------------#
  • I don't understand why people keep all kinds of status flags and hold off until the end of a routine to "return". If you know that you will do nothing after a failure at:

          if (!$dbh) {
                    $ok = 0;
                    $msg = "Failed to get connection to databasename in GetValues";
            } else {
                    $sth = $dbh->prepare('Se
  • ick (Score:3, Interesting)

    by gav (2710) on 2003.09.17 12:00 (#24273) Homepage Journal

    I find code like this pretty cringe-worthy. If you set RaiseError and HandleError, you can have something this simple:

    sub get_values {
        my $dbh = get_connection('databasename');
        return $dbh->selectall_arrayref(
            'SELECT * FROM table_name', { Slice => {} }
        );
    }

    Isn't that much nicer than 25 lines of code? I'm also not keen on returning flags for errors, you'll just have to write lots of code to check, just die if something bad happened and trap it with eval.

    • Yes, and most of the time, the correct reaction to something bad happening is to die, so you can even drop the eval.
  • The best way I've found is to use RaiseError => 1, which tells DBI to always throw an exception when there is an error. This way, you never have errors go unnoticed. If you want an error to not be fatal, use eval. eval { $dbh->do("DROP TABLE users"); }; if ( $@ ) { my_error_handler( $@ ) } Another method I use is a function that does a little more digging for me before printing it's error message. sub check_dbi_errors ($) { my $sth = shift; return if (!$sth-
    • Hmm, I didn't look and see that the pre tag isn't allowed. bummer.

      The best way I've found is to use RaiseError => 1, which tells DBI to always throw an exception when there is an error. This way, you never have errors go unnoticed. If you want an error to not be fatal, use eval.

      eval {
            $dbh->do("DROP TABLE users");
      };
      if ( $@ ) { my_error_handler( $@ ) }

      Another method I use is a function that does a little more digging for me before printing it's error message.

      sub check_dbi_erro