http://qs1969.pair.com?node_id=417872

[ Update: let me retrospectively preface this rant with a note that I used to a huge fan of the technique I am deprecating. My disdain has grown organically. There is no single particularly striking reason why it is bad; I have just experienced that trying to work with code that exploits this technique is a severe pain in practice for a number of subtle reasons that combine to bite you, never fatally, but persistently. ]

When seeking advice about your OO Perl style, you may find people from time to time who will encourage you to write mutators (aka setters) such that they will return $self:

sub set_frob {
    my $self = shift;
    my ( $val ) = @_;
    if( @_ == 1 and $val !~ /\D/ ) {
        $self->{ frob } = $val;
    }
    else {
        die $some_param_inval_explanation;
    }
    return $self; # this is all that matters to us
}

A number of popular modules like Tk and SOAP::Lite use this kind of interface. This, proponents argue, will allow you to conveniently chain mutator calls on a single object:

$frobnitzer->set_frob( 42 ) ->set_flub( 712 ) ->set_name( 'veeblefetzer' );

Please don't.


First of all, this creates temptation to mix in calls to setters with method calls with actual return values:

my $label = $button->border( 20 )->label;

This can be pretty confusing to read. Even if you don't think so at this point, this is not at all uncommon in practice, with modules like Data::Dumper actively promoting this kind of interface.

But it gets a whole lot worse yet when the object in question actually aggregates others and lets you ask for them. This introduces multiple different kinds of possible call chains:

$window->title( 'foo' )->border( 20 ); # semantically very different from: $window->child->border( 20 );

Suddenly the observer has to read carefully to keep affairs straight.

In the worst case, mixing all possible scenarios, you get code like this:

my $label_formatting = $window->title( 'foo' ) ->border( 20 ) ->child->border( 20 ) ->child->font_style;

If that's not ugly as all sin, you tell me what is. Transgressions like this are rare, to be sure (and thankful).

But $self-returning mutators blur the lines between types of methods and types of call chains, and ultimately they'll make your code harder to read.

[ Update: Separate things should be kept separate:

$window->title( 'foo' ); $window->border( 20 ); my $button = $window->child; $button->border( 20 ); my $label_formatting = $button->child->font_style;

This is more code, but it is worlds apart in clarity.

[ Update: Note how this codes makes it possible to discriminate between errors from the one mutator vs those from another mutator. In the chained calls version of the code, doing so would require many more provisions. And how do you handle error signalling by the return value of a method? You can't. There are tricks like returning a magic object that can have any method called on it and overloads its boolean value to be false, but… where's that smell coming from? ]

If you're writing a class, please resist the temptation to facilitate chained mutator calls. If you are looking for a sensible return value for your mutators, return the current value of the property, which is analogous to how $foo = $bar = $baz works. ]

[ Update: but better yet, don't write separate mutators per property at all. Write a single method that can update any number of properties in one fell swoop:

$window->set( title => 'foo', border => 20, );

As a bonus, you get to avoid writing repitiuous code or harrowing with code generation schemes, and all your input validation will naturally converge on a single spot in your code without the additional effort this would require with the separate mutators model. ]

Makeshifts last the longest.