Should a Perl constructor return an undef or a "invalid" object?

Question

Question:

What is considered to be "Best practice" - and why - of handling errors in a constructor?.

"Best Practice" can be a quote from Schwartz, or 50% of CPAN modules use it, etc...; but I'm happy with well reasoned opinion from anyone even if it explains why the common best practice is not really the best approach.

As far as my own view of the topic (informed by software development in Perl for many years), I have seen three main approaches to error handling in a perl module (listed from best to worst in my opinion):

1. Construct an object, set an invalid flag (usually "is_valid" method). Often coupled with setting error message via your class's error handling.

   Pros:

   • Allows for standard (compared to other method calls) error handling as it allows to use $obj->errors() type calls after a bad constructor just like after any other method call.

   • Allows for additional info to be passed (e.g. >1 error, warnings, etc...)

   • Allows for lightweight "redo"/"fixme" functionality, In other words, if the object that is constructed is very heavy, with many complex attributes that are 100% always OK, and the only reason it is not valid is because someone entered an incorrect date, you can simply do "$obj->setDate()" instead of the overhead of re-executing entire constructor again. This pattern is not always needed, but can be enormously useful in the right design.

   Cons: None that I'm aware of.

2. Return "undef".

   Cons: Can not achieve any of the Pros of the first solution (per-object error messages outside of global variables and lightweight "fixme" capability for heavy objects).

3. Die inside the constructor. Outside of some very narrow edge cases, I personally consider this an awful choice for too many reasons to list on the margins of this question.

4. UPDATE: Just to be clear, I consider the (otherwise very worthy and a great design) solution of having very simple constructor that can't fail at all and a heavy initializer method where all the error checking occurs to be merely a subset of either case #1 (if initializer sets error flags) or case #3 (if initializer dies) for the purposes of this question. Obviously, choosing such a design, you automatically reject option #2.

Answer 1

It depends on how you want your constructors to behave.

The rest of this response goes into my personal observations, but as with most things Perl, Best Practices really boils down to "Here's one way to do it, which you can take or leave depending on your needs." Your preferences as you described them are totally valid and consistent, and nobody should tell you otherwise.

I actually prefer to die if construction fails, because we set it up so that the only types of errors that can occur during object construction really are big, obvious errors that should halt execution.

On the other hand, if you prefer that doesn't happen, I think I'd prefer 2 over 1, because it's just as easy to check for an undefined object as it is to check for some flag variable. This isn't C, so we don't have a strong typing constraint telling us that our constructor MUST return an object of this type. So returning undef, and checking for that to establish success or failure, is a great choice.

The 'overhead' of construction failure is a consideration in certain edge cases (where you can't quickly fail before incurring overhead), so for those you might prefer method 1. So again, it depends on what semantics you've defined for object construction. For example, I prefer to do heavyweight initialization outside of construction. As to standardization, I think that checking whether a constructor returns a defined object is as good a standard as checking a flag variable.

EDIT: In response to your edit about initializers rejecting case #2, I don't see why an initializer can't simply return a value that indicates success or failure rather than setting a flag variable. Actually, you may want to use both, depending on how much detail you want about the error that occurred. But it would be perfectly valid for an initializer to return true on success and undef on failure.

Answer 2

I prefer:

1. Do as little initialization as possible in the constructor.
2. croak with an informative message when something goes wrong.
3. Use appropriate initialization methods to provide per object error messages etc

In addition, returning undef (instead of croaking) is fine in case the users of the class may not care why exactly the failure occurred, only if they got a valid object or not.

I despise easy to forget is_valid methods or adding extra checks to ensure methods are not called when the internal state of the object is not well defined.

I say these from a very subjective perspective without making any statements about best practices.

Answer 3

I would recommend against #1 simply because it leads to more error handling code which will not be written. For example, if you just return false then this works fine.

my $obj = Class->new or die "Construction failed...";

But if you return an object which is invalid...

my $obj = Class->new;
die "Construction failed @{[ $obj->error_message ]}" if $obj->is_valid;

And as the quantity of error handling code increases the probability of it being written decreases. And its not linear. By increasing the complexity of your error handling system you actually decrease the amount of errors it will catch in practical use.

You also have to be careful that your invalid object in question dies when any method is called (aside from is_valid and error_message) leading to yet more code and opportunities for mistakes.

But I agree there is value in being able to get information about the failure, which makes returning false (just return not return undef) inferior. Traditionally this is done by calling a class method or global variable as in DBI.

my $dbh = DBI->connect($data_source, $username, $password) or die $DBI::errstr;

But it suffers from A) you still have to write error handling code and B) its only valid for the last operation.

The best thing to do, in general, is throw an exception with croak. Now in the normal case the user writes no special code, the error occurs at the point of the problem, and they get a good error message by default.

my $obj = Class->new;

Perl's traditional recommendations against throwing exceptions in library code as being impolite is outdated. Perl programmers are (finally) embracing exceptions. Rather than writing error handling code ever and over again, badly and often forgetting, exceptions DWIM. If you're not convinced just start using autodie (watch pjf's video about it) and you'll never go back.

Exceptions align Huffman encoding with actual use. The common case of expecting the constructor to just work and wanting an error if it doesn't is now the least code. The uncommon case of wanting to handle that error requires writing special code. And the special code is pretty small.

my $obj = eval { Class->new } or do { something else };

If you find yourself wrapping every call in an eval you are doing it wrong. Exceptions are called that because they are exceptional. If, as in your comment above, you want graceful error handling for the user's sake, then take advantage of the fact that errors bubble up the stack. For example, if you want to provide a nice user error page and also log the error you can do this:

eval {
    run_the_main_web_code();
} or do {
    log_the_error($@);
    print_the_pretty_error_page;
};

You only need it in one place, at top of your call stack, rather than scattered everywhere. You can take advantage of this at smaller increments, for example...

my $users = eval { Users->search({ name => $name }) } or do {
    ...handle an error while finding a user...
};

There's two things going on. 1) Users->search always returns a true value, in this case an array ref. That makes the simple my $obj = eval { Class->method } or do work. That's optional. But more importantly 2) you only need to put special error handling around Users->search. All the methods called inside Users->search and all the methods they call... they just throw exceptions. And they're all caught at one point and handled the same. Handling the exception at the point which cares about it makes for much neater, compact and flexible error handling code.

You can pack more information into the exception by croaking with a string overloaded object rather than just a string.

my $obj = eval { Class->new }
  or die "Construction failed: $@ and there were @{[ $@->num_frobnitz ]} frobnitzes";

Exceptions:

• Do the right thing without any thought by the caller
• Require the least code for the most common case
• Provide the most flexibility and information about the failure to the caller

Modules such as Try::Tiny fix most of the hanging issues surrounding using eval as an exception handler.

As for your use case where you might have a very expensive object and want to try and continue with it partially build... smells like YAGNI to me. Do you really need it? Or you have a bloated object design which is doing too much work too early. IF you do need it, you can put the information necessary to continue the construction in the exception object.