http://qs321.pair.com?node_id=648277

I've seen plenty of questions about returning two values or returning a value and doing something as well... in this small piece of code in the Class::Base docs we see:
# let's make 'foobar' a mandatory argument $self->{ foobar } = $config->{ foobar } || return $self->error("no foobar argument");
and the beauty is in return $self->error("no foobar argument");

Of course, the beast is the use of || instead of or, but let's sweep that under the rug.

Notice how his method gets two things done for him. He sets the value of the error for later inspection by the caller. And then the error method returns undef as a way of signalling failure.

Yet another slik tekneek for us 2 beehold.


Carter's compass: I know I'm on the right track when by deleting something, I'm adding functionality

Replies are listed 'Best First'.
Re: getting 2 things done at once with Class::Base
by jZed (Prior) on Oct 31, 2007 at 18:46 UTC
    This technique is heavily used in DBI internals. Look at any DBD and you'll see lots of things like:
    return $dbh->set_err( $DBI::stderr, "Invalid attribute '$attrib'" );
    which sets the error to be acted on by RaiseError or PrintError or called explicitly with $h->errstr() and then returns undef.
Re: getting 2 things done at once with Class::Base
by Mutant (Priest) on Oct 31, 2007 at 17:02 UTC
    For parameter errors, I prefer to throw an exception (well, a croak, which is as close to an exception as Perl gets). It prevents the caller from making stupid mistakes (like not passing that parameter at all), but still allows them to wrap it in an eval{} if they need to gracefully handle edge cases where that parameter wasn't set for some reason. (Even in those cases, they probably want to know about the error somehow, because it's likely a bug. This parameter is mandatory, so presumably we can't do anything without it).
Re: getting 2 things done at once with Class::Base
by dragonchild (Archbishop) on Oct 31, 2007 at 15:03 UTC
    I seriously hope that he's not doing a return undef; because that evaluates as true in list context.

    My criteria for good software:
    1. Does it work?
    2. Can someone else come in, make a change, and be reasonably certain no bugs were introduced?

      If the method only ever returns a single scalar, then returning undef is better than returning an empty list. Each has its pitfalls. Sure, undef in a list assignment in a Boolean context isn't false. But an empty list as part of lots of constructs where a single scalar is expected leads to surprises that are at least as bad.

      For subs that at some other point can return more than one item, then I agree that one should return an empty list to indicate "failure" not undef. For subs that in all other cases return a single scalar, it is better to return undef than to transform your function into something that returns a single scalar except in the case of failure.

      I've yet to see what I would consider reasonable code that demonstrates using a function that always returns a single scalar in a list assignment and using that list assignment in a Boolean context and expecting failure to be indicated. So for such functions, the theorized risk from returning undef is nearly zero.

      But I've seen lots of people bitten by functions returning an empty list when what they expected was a single scalar that might be undef. This problem happens for any inclusion of such in a list like an array or hash initialization, a call to almost any function (including things like push), etc. I find such code rather frequent and reasonable so I think mitigating this risk is much more important.

      - tye        

        According to the docs (Perlfunc) on return:

        If no EXPR is given, returns an empty list in list context, the undefined value in scalar context, and (of course) nothing at all in a void context.

        I think this is mentioned in Perl Best Practices. I don't have a copy handy to verify it. So, the better practice seems to be to replace return undef; with return;.

        A quick test shows that this works fine for boolean tests as well. We get the best of all worlds.

        print "foo\n" unless foo(); sub foo { return; }
        HTH,
        Charles
      Whatever he is doing, I completely do _not_ understand it: He is returning a dereference of $errvar.
      #--------------------------------------------------------------------- +--- # error() # error($msg, ...) # # May be called as a class or object method to set or retrieve the # package variable $ERROR (class method) or internal member # $self->{ _ERROR } (object method). The presence of parameters indic +ates # that the error value should be set. Undef is then returned. In the # abscence of parameters, the current error value is returned. #--------------------------------------------------------------------- +--- sub error { my $self = shift; my $errvar; { # get a reference to the object or package variable we're munging no strict qw( refs ); $errvar = ref $self ? \$self->{ _ERROR } : \${"$self\::ERROR"}; } if (@_) { # don't join if first arg is an object (may force stringification) $$errvar = ref($_[0]) ? shift : join('', @_); return undef; } else { return $$errvar; } }


      Carter's compass: I know I'm on the right track when by deleting something, I'm adding functionality

        He returns undef unless no arguments were given. Not particularly complex code. Even documented:

        The presence of parameters indicates [...]. Undef is then returned. In the abscence of parameters­, the current error value is returned.

        His comments would be clearer if he changed one period into a semicolon, however.

        - tye