Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

What should I do if a Moose builder method fails?

Tags:

perl

moose

What is the best way to handle a failure in a builder method?

For example:

package MyObj;
use Moose;
use IO::File;

has => 'file_name'   ( is => 'ro', isa => 'Str',      required   =>1  );
has => 'file_handle' ( is => 'ro', isa => 'IO::File', lazy_build => 1 );

sub _build_file_handle {
    my $self = shift;
    my $fh = IO::File->new( $self->file_name, '<' );

    return $fh;
}

If the _build_file_handle fails to get a handle, the builder will return undef, which fails the type constraint.

I could use a union in the file_handle type constraint, so that it will accept an undef as a valid value. But then, the predicate has_file_handle will return true, even when the value is undef.

Is there a way to signal that the builder failed, and the attribute should remain cleared?

like image 911
daotoad Avatar asked Jan 29 '10 19:01

daotoad


2 Answers

You are not thinking at a high-enough level. OK, the builder fails. The attribute remains undefined. But what do you do about the code that is calling the accessor? The class' contract indicated that calling the method would always return an IO::File. But now it's returning undef. (The contract was IO::File, not Maybe[IO::File], right?)

So on the next line of code, the caller is going to die ("Can't call method 'readline' on an undefined value at the_caller.pl line 42."), because it expects your class to follow the contract that it defined. Failure was not something your class was supposed to do, but now it did. How can the caller do anything to correct this problem?

If it can handle undef, the caller didn't actually need a filehandle to begin with... so why did it ask your object for one?

With that in mind, the only sane solution is to die. You can't meet the contract you agreed to, and die is the only way you can get out of that situation. So just do that; death is a fact of life.

Now, if you aren't prepared to die when the builder runs, you'll need to change when the code that can fail runs. You can do it at object-construction time, either by making it non-lazy, or by explicitly vivifying the attribute in BUILD (BUILD { $self->file_name }).

A better option is to not expose the file handle to the outside world at all, and instead do something like:

# dies when it can't write to the log file
method write_log {
    use autodie ':file'; # you want "say" to die when the disk runs out of space, right?
    my $fh = $self->file_handle;
    say {$fh} $_ for $self->log_messages;
}

Now you know when the program is going to die; in new, or in write_log. You know because the docs say so.

The second way makes your code a lot cleaner; the consumer doesn't need to know about the implementation of your class, it just needs to know that it can tell it to write some log messages. Now the caller is not concerned with your implementation details; it just tells the class what it really wanted it to do.

And, dying in write_log might even be something you can recover from (in a catch block), whereas "couldn't open this random opaque thing you shouldn't know about anyway" is much harder for the caller to recover from.

Basically, design your code sanely, and exceptions are the only answer.

(I don't get the whole "they are a kludge" anyway. They work the exact same way in C++ and very similarly in Java and Haskell and every other language. Is the word die really that scary or something?)

like image 52
jrockway Avatar answered Sep 21 '22 13:09

jrockway


"Best" is subjective, but you'll have to decide which makes more sense in your code:

  1. if you can continue on in your code when the filehandle fails to build (i.e. it is a recoverable condition), the builder should return undef and set the type constraint to 'Maybe[IO::File]'. That means you'll also have to check for definedness on that attribute whenever using it. You could also check if this attribute got built properly in BUILD, and choose to take further action at that point (as friedo alluded to in his comment), e.g. calling clear_file_handle if it is undef (since a builder will always assign a value to the attribute, assuming it doesn't die of course).

  2. otherwise, let the builder fail, either by explicitly throwing an exception (which you can opt to catch higher up), or simply returning undef and letting the type constraint fail. Either way your code will die; you just get a choice of how it dies and how voluminous the stack trace is. :)

PS. You may also want to look at Try::Tiny, which Moose uses internally, and is basically just a wrapper for* the do eval { blah } or die ... idiom.

*But done right! and in a cool way! (I seem to hear lots of whispering in my ear from #moose..)

like image 39
Ether Avatar answered Sep 17 '22 13:09

Ether