As convincingly argued in Perl Best Practices, Chapter 13 (e.g. first guideline is "Throw exceptions instead of returning special values or setting flags"), generally you should embrace exception handling.

However, having each method simply catch and rethrow exceptions does seem evil ... or at least pointless. Maybe there was a historical reason for this? Is there technical documentation for this system describing why this is being done and, more importantly, describing a rational and consistent error handling policy for the system as a whole?

Generally, throwing should be done when a method detects an error that it cannot resolve itself and catching should be done where there is sufficient knowledge/context to sensibly handle/report the error. Catching just to rethrow seems warranted only at "system boundaries" (e.g. translating a low level error message into a higher-level one); if a method isn't going to handle, translate, or intentionally ignore the error itself, it should simply let it propagate upwards to a caller who can.

You might be able to install Carp's confess method as your fatal error handler. Then you can eliminate the evals and checking code.

#!/usr/bin/perl
#
# Die with backtrace
#
use Carp;
$SIG{__DIE__} = \&Carp::confess;

callMethod();

sub callMethod
{
  # blah blah
  someMethod(0);
}

Sub someMethod {
  1/shift;
}
[download]

sub callMethod
{
  local $SIG{__DIE__} = sub {confess ("error calling someMethod: $_[0]
+")};
  someMethod(0);
}
[download]

If you really want to fix things, investigate the all possible causes of exceptions and decide ahead of time how you want to handle them. Then stop and consider if that really provides any benefit over how things are now.

I'm leaving them for now, but since this is a daemon, I think (though have yet to benchmark) that removing the evals will speed things up a little. Or a lot. If a lot, removing them suddenly becomes essential.

I was just wondering right now if philosophically it was better to work towards removing them.

Hmmmm.

You think that block evals are a performance penalty? I've never heard that. Have you tested that theory at all?

use strict;
use warnings;
use Benchmark 'cmpthese';

cmpthese(10000, {
    'eval'   => sub { eval_code() },
    'noeval' => sub { no_eval_code() },,
});

sub eval_code {
    my $x = 0;
    for (1..1000) {
        eval {
            $x+=1;
        }
    }
}

sub no_eval_code {
    my $x = 0;
    for (1..1000) {
        $x+=1;
    }
}
[download]

         Rate   eval noeval
eval   1495/s     --   -62%
noeval 3953/s   164%     --
[download]

use strict;
use warnings;
use Benchmark 'cmpthese';

cmpthese(1000000, {
    'eval'   => sub { eval_code() },
    'noeval' => sub { no_eval_code() },,
});


sub eval_code {
    my $x = 0;
    for (reverse 0..9) {
        eval {
            my $x = 100/$_;
        };
        if ($@) {
            error_sub();
        }
    }
}

sub no_eval_code {
    for (reverse 0..9) {
        if ($_==0) {
            error_sub();
        }
        else {
            my $x = 100/$_;
        }
    }
}
[download]

           Rate   eval noeval
eval    67797/s     --   -39%
noeval 111111/s    64%     --
[download]

Exceptions should be "exceptional". That is, you shouldn't expect them to happen during normal functioning of the program. So lets see what the overhead of providing (unused) exception handling is:

use strict;
use warnings;
use Benchmark 'cmpthese';

cmpthese(-1, {
    'eval'   => \&eval_code,
    'noeval' => \&no_eval_code,
});

sub eval_code {
    my $x = 0;
    for (reverse 0..9) {
        eval {
            some_more_code ($_);
        };
        if ($@) {
            error_sub();
        }
    }
}

sub no_eval_code {
    for (reverse 0..9) {
        if ($_==0) {
            error_sub();
        }
        else {
            some_more_code ($_);
        }
    }
}

sub some_more_code {
}

sub error_sub {
}
[download]

Prints:

           Rate   eval noeval
eval   185563/s     --    -7%
noeval 198639/s     7%     --
[download]

In words: the processing cost of an unexercised eval is essentially nothing.

---

DWIM is Perl's answer to Gödel

Hmmm, well, I stripped it down further:

use strict;
use warnings;
use Benchmark 'cmpthese';

cmpthese(-1, {
    'eval'   => \&eval_code,
    'noeval' => \&no_eval_code,
 });

 sub eval_code {
     eval {
        some_more_code ();
     };
     if ($@) {
         error_sub();
     }
}

sub no_eval_code {
    if (0) {
        error_sub();
    }
    else {
        some_more_code ();
    }
}

sub some_more_code { }
sub error_sub { }
[download]

And got this (I think the effectively empty eval is meaningless - see here where I ran it 4 times):

            Rate   eval noeval
eval   1298354/s     --   -37%
noeval 2066450/s    59%     --

            Rate   eval noeval
eval   1316991/s     --   -38%
noeval 2123851/s    61%     --

            Rate   eval noeval
eval   1217925/s     --   -21%
noeval 1534287/s    26%     --

            Rate   eval noeval
eval   1298354/s     --   -41%
noeval 2205537/s    70%     --
[download]

I have a feeling an empty eval gets optimized away at compile time (though I am most probably wrong :)

How about if I add in an increment do ensure each eval has to be 'eval'd, ie

use strict;
use warnings;
use Benchmark 'cmpthese';

my $x;

cmpthese(-1, {
    'eval'   => \&eval_code,
    'noeval' => \&no_eval_code,
 });

sub eval_code {
     $x=1;
     eval {
        some_more_code ();
     };
     if ($@) {
         error_sub();
     }
}

sub no_eval_code {
    if ($x==0) {
        error_sub();
    }
    else {
        $x=0;
        some_more_code ();
    }
}

sub some_more_code { $x++ }
sub error_sub { }
[download]

I get this:

            Rate   eval noeval
eval    989400/s     --   -23%
noeval 1286220/s    30%     --
-bash-3.00$ perl tmp.pl
            Rate   eval noeval
eval   1092266/s     --   -21%
noeval 1379705/s    26%     --

            Rate   eval noeval
eval   1069351/s     --   -19%
noeval 1327443/s    24%     --

            Rate   eval noeval
eval   1071850/s     --   -25%
noeval 1435550/s    34%     --

            Rate   eval noeval
eval   1071850/s     --   -26%
noeval 1456355/s    36%     --
[download]

Either way, I still haven't been able to create a counter example that shows eval to be faster.

But anyway, considering the number of runs a second, I have a feeling my energies are probably better spent focusing on other parts of the code as far as optimizations are concerned :)

I would expect eval to be a little slower, but not that much. Maybe this is because of the extra scope you're adding here. Try changing the no_eval_code to this, to see if the extra scoping is free or not:

sub no_eval_code {
    for (reverse 0..9) {
        if ($_==0) {
            error_sub();
        }
        else {
            {
                my $x = 100/$_;
            }
        }
    }
}
[download]

heh

           Rate   eval noeval
eval    88768/s     --   -39%
noeval 146161/s    65%     --
[download]

Consider

eval {
   my ($dev, $inode) = stat($dir) or die "stat $dir\n";
   eval {
       opendir D, $dir or die "opendir $dir\n";
       eval {
            my $e = readdir(D) or die "readdir $dir\n";
            eval {
                open my $in, '<' $e or die "open $e\n";
                # etc etc
            }
       }
   }
}
[download]

Especially if there was lots of other code intertwined within those blocks, when sprinkled with lots of conditionals on, and assignments to, $@, to confuse the unwary. Now that's what I would call evil nested evals.

Your puny evilness cannot hope to compare with this evilness (which is what I was expecting to find when I read the title of this node). Your code is naught but exception handling, which is not so much evil as merely vile :)

This isn't what I'd call "nesting", and yes it is a little redundant. But, it's also harmless.

I'd guess that someone developed callMethod and did some cargo-cult error handling. There's not much point in catching an exception only to throw it again, but perhaps the coder planned on adding some additional handling code.

I'd further guess that later on, some developer (maybe the same one, maybe not) called callMethod, knowing only that it threw an error, and so caught it, and made the same cargo-cult mistake.

If all those various catching blocks (the if ($@) bits) are only re-throwing the error, then they are kind of pointless and redundant. They could probably be removed.

On the other hand, catching an exception and doing something useful about it (writing additional detail to the logs, asking the user what to do, retrying, etc.) is a good pattern, esp. for user-interactive programs. Always do the least surprising thing -- that is, whatever will least surprise your user.