Perl Antipattern #3.14159

ziggy on 2002-03-12T20:45:48

Here's a common antipattern in Perl code.

In lower level languages, aggregate data types are not first class citizens. For example, C programmers pass arrays around by passing pointers -- meaning that the "copy" is actually a reference to the same block of data, and is therefore mutable. Frequently, the intent is to copy the entire array, not a pointer to the array. To do this, it's necessary to rigorously construct a copy of that array and pass a reference to that copy.

In Perl, it's possible to create copies of aggregate datatypes, as well as create references. If you really and truly want to work with references to the same exact block of data, that's possible. However, the more common context is to create a copy. Thankfully, because aggregates are first class datatypes in Perl, this isn't the same tedious chore it would be in C or C++.

An example: given a set of values in a hash, initialize another hash by copying those values over. Here is a classic way to begin:

my %defaults = @_;
## ... stuff happens ...
my %hash;

## Seed the values in %hash with values from %defaults
foreach (keys %defaults) {
    $hash{$_} = $defaults {$_};
}
Of course there are ways to reduce a line or two out of that if you're playing golf. :-) Realistically, this is as simple as this antipattern can get in the wild; occasionally, lots of other extraneous and useless statements (and comments!) are shoved in this overused construct.

Now, suppose you had a restricted set of values you wanted to copy from %defaults. One way to do it would be to change the list of keys to iterate over in the foreach. Or, perhaps do something a little more advanced, like this:

my %defaults = @_;
## ... stuff happens ...
my %hash;

## Seed the values in %hash with values from %defaults
%hash = map {$_ => $defaults{$_}} qw(a b c d);
Still gets the job done, but still too much work.

Remember, aggregates are first class values in Perl. A hash can be assigned all of it's values at once in list context. Here again is the simple case, when we want to copy all of the values into a new hash:

my %defaults = @_;
## ... stuff happens ...
my %hash = %defaults;
That's it. We're done. Except for the case when we want to assign a specific set of values from %defaults to our hash:
my %defaults = @_;
## ... stuff happens ...
my %hash;
my @keys = qw(a b c d);
@hash{@keys} = @defaults{@keys};

And now we're really done. Aren't hash slices handy? :-) Of course, if we needed to do a more compliated assignment, such as redefining the key names in %hash when we copy them from %defaults, then we probably want to use a hash slice assignment to perform the mapping. Otherwise the code just gets too nasty.


I do something similar

ajtaylor on 2002-03-13T02:40:03

My constructors are a combination of your last two examples. The advantage is that you know what your expected keys are, your defaults are set w/o an extra hash, and the whole thing is quite simple. :-)

my %args = (default1=>'1', default2=>'2', @_);

Re:I do something similar

ziggy on 2002-03-13T03:12:05

That's certainly a valid way to proceed. The code I was reacting to was a constructor that looked something like this:
sub new {
    my $class = shift;
    my %params = @_;

    my $self = {};
    bless $self, $class;
    foreach (keys %params) {
        $self->{$_} = $params{$_};
    }

    return $self;
}
It's obviously a very common idiom, but taking 10 lines to say it obscures the intent.

Re:I do something similar

ajtaylor on 2002-03-14T05:22:49

I split up the construction and initialization. new() usually just creates the object, and then calls init() to do the initialization. And I use the nifty map construct "map {$self->{$_} = $_} keys %args" instead of the foreach.

Re:I do something similar

koschei on 2002-03-14T12:30:20

Maybe I'm being obtuse, but why not:

@{$self}{keys %args} = values %args;

I'm mostly thinking of getting rid of obvious iterators.

Personally, I'd validate that %args has appropriate arguments in it. Otherwise you could be stuffing anything in there.

Re:I do something similar

ajtaylor on 2002-03-14T16:58:49

Your way works fine, but mine is easier to read. That might not be an issue if your coworkers are experienced with perl. But based on some of the code I've seen at my new job, cool constructs like yours would be line noise.

And yes, validation is good. But I'm only using this for internal use, so I trust my other code is OK. :-)

Re:I do something similar

pudge on 2002-03-14T17:09:54

I have coworkers who don't grok such constructs. I tell them if they don't understand it to come talk to me. Hash slices is one example of things they sometimes don't understand. If it is especially odd I will comment it a bit. But I am always willing to explain constructs to them that they are unfamiliar with.

Re:I do something similar

koschei on 2002-03-14T17:25:30

This is more my philosophy. I'd rather any co-workers learn more (better?) Perl than dumb down my coding.

I'm all for ease of reading, but to the trained eye, which is actually more understandable? It took me less time to grok my version than to fathom the various alternatives.

You can also sometimes see a flip side - write something too simply without using language features and experienced people will have trouble with what was written - either thinking that there must be some catch or that it could have been expressed far simpler =)

So, yes, I agree to everyone =)

Re:I do something similar

ajtaylor on 2002-03-14T18:18:56

Excellent point! I've always been keen for teaching/learning. It's not so much an issue at my employer as I am the second programmer and I suspect I might have a more in-depth knowledge of perl than my superior. But I've only been here 4 days, so what do I know! ;-)