tags:

views:

218

answers:

6

I have a Perl routine that manages error checking. There are about 10 different checks and some are nested, based on prior success. These are typically not exceptional cases where I would need to croak/die. Also, once an error occurs, there's no point in running through the rest of the checks.

However, I can't seem to think of a neat way to solve this issue except by using something analogous to the following horrid hack:

sub lots_of_checks
{

 if(failcond)
 {
  goto failstate:
 }
 elsif(failcond2)
 {
  goto failstate;
 }

 #This continues on and on until...

 return 1; #O happy day!

 failstate:

 return 0; #Dead...
}

What I would prefer to be able to do would be something like so:

do
{
 if(failcond)
 {
  last;
 }
 #...
};
+8  A: 

Perhaps you want to have a look at the following articles about exception handling in perl5:

Inshallah
+11  A: 

An empty return statement is a better way of returning false from a Perl sub than returning 0. The latter value will actually be true in list context:

sub lots_of_checks {
    return if fail_condition_1;
    return if fail_condition_2;
    # ...
    return 1;
}
Sinan Ünür
Is that identical to saying `return undef if cond;` ?
Paul Nathan
@Paul, a blank return gives undef in scalar context and an empty list is list context.
daotoad
+4  A: 

Given your example, I'd write it this way:

sub lots_of_checks {
    local $_ = shift;  # You can use 'my' here in 5.10+

    return if /condition1/;
    return if /condition2/;
    # etc.

    return 1;
}

Note the bare return instead of return 0. This is usually better because it respects context; the value will be undef in scalar context and () (the empty list) in list context.

If you want to hold to a single-exit point (which is slightly un-Perlish), you can do it without resorting to goto. As the documentation for last states:

... a block by itself is semantically identical to a loop that executes once. Thus "last" can be used to effect an early exit out of such a block.

sub lots_of_checks {
    local $_ = shift;
    my $all_clear;

    {
        last if /condition1/;
        last if /condition2/;
        # ...
        $all_clear = 1; # only set if all checks pass
    }

    return unless $all_clear;
    return 1;
}
Michael Carman
Don't do `local $_;` Use `local $*;` I can't locate the reference right now but I am sure brian d foy will be able to cite it.
Sinan Ünür
From perlmonks: local $_ doesn't work well if $_ is tied or aliased to someting that's tied, and it doesn't protect pos($_). In this case, `local $_ = shift;` sufficiently unusual that I would prefer `my ($arg) = @_; for ( $arg ) { /condition1/ etc etc }` given that you have to use curlies anyway.
Sinan Ünür
I avoided the pseudo-switch syntax thinking that it would make the mechanism in the second example clearer by preserving symmetry with the first example. As for localizing `$_`, it's unfortunate that the behavior of `local` is badly entangled with typeglobs. I've run into problems before trying to localize exported package variables. The "safe" (and ugly) alternative is to export/localize the entire glob. What's the implementation in a loop -- `local $_` or `local $*`? I can see either one causing problems...
Michael Carman
@Michael Carman Well, unfortunately, all I know is that there may be problems with `local` used that way. In any case, I try to avoid `$_` and stick with lexical variables in most cases mainly because I am too lazy to look up these sorts of issues.
Sinan Ünür
@Sinan Unur, very interesting points. Can you provide a link to the discussion on PM? I've used and advocated using 'local $_`. I'd like to know all I can. Thanks.
daotoad
@Sinan Unur: I think you meant `local *_` instead of `local $*`
Michael Carman
@daotoad I finally located it. http://www.perlmonks.org/?node_id=561931 Incidentally, the context in which it came up is interesting as well: http://www.perlmonks.org/?node_id=561895
Sinan Ünür
@Michael Carman Absolutely, I mean `local *_`. Too bad comments cannot be edited.
Sinan Ünür
+5  A: 

You absolutely can do what you prefer.

Check: {
    last Check
        if failcond1;
    last Check
        if failcond2;
    success();
}
chaos
+2  A: 

If you want to keep your single in/single out structure, you can modify the other suggestions slightly to get:

sub lots_of_checks
{
    goto failstate if failcond1;

    goto failstate if failcond2;

    # This continues on and on until...

    return 1; # O happy day!

    failstate:

    # Any clean up code here.

    return; # Dead...
}

IMO, Perl's use of the statement modifier form "return if EXPR" makes guard clauses more readable than they are in C. When you first see the line, you know that you have a guard clause. This feature is often denigrated, but in this case I am quite fond of it.

Using the goto with the statement modifier retains the clarity, and reduces clutter, while it preserves your single exit code style. I've used this form when I had complex clean up to do after failing validation for a routine.

daotoad
+5  A: 

Why would you not use exceptions? Any case where the normal flow of the code should not be followed is an exception. Using "return" or "goto" is really the same thing, just more "not what you want".

(What you really want are continuations, which "return", "goto", "last", and "throw" are all special cases of. While Perl does not have full continuations, we do have escape continuations; see http://search.cpan.org/~sartak/Continuation-Escape-0.03/lib/Continuation/Escape.pm)

In your code example, you write:

do
{
 if(failcond)
 {
  last;
 }
 #...
};

This is probably the same as:

eval {
   if(failcond){
       die 'failcond';
   }
}

If you want to be tricky and ignore other exceptions:

my $magic = [];
eval {
    if(failcond){
        die $magic;
    }
}
if ($@ != $magic) {
    die; # rethrow
}

Or, you can use the Continuation::Escape module mentioned above. But there is no reason to ignore exceptions; it is perfectly acceptable to use them this way.

jrockway