Pointless temporary variables

gav on 2004-07-14T19:37:41

One of my pet peeves is pointless temporary variables, they often just clutter things up. Consider this example (pulled from production code) that is crying out for some refactoring.

my $keywords;
@$keywords = split ' ', $something;
foreach my $word (@$keywords) {
    # ...
}

vs.

foreach my $word (split ' ', $something) {
    # ...
}

Isn't this much easier to understand?


maybe...

jdavidboyd on 2004-07-14T19:54:59

the original author wanted to be able to view @$keywords in a debugger, to see the result of the split.

I've split up operations sometimes, placing an intermediary step in between, to facilitate debugging.

Just a thought.

Re:maybe...

vsergu on 2004-07-14T21:30:12

But why @$keywords instead of plain old @keywords? What's the point of the reference?

Re:maybe...

jdavidboyd on 2004-07-15T14:10:10

Hmm, good point. Oh well, it was just a swag, anyway...

Depends on who's reading it

cbrandtbuffalo on 2004-07-15T00:46:26

Both examples make sense to me, but if you've got Perl coders who don't code Perl *all* the time, I think the second example would require a few more brain cycles than the the first. This is especially true if there's one part they don't remember--like the fact that split returns a list. The temporary variable re-enforces this by showing it. Besides, memory is cheap :)

Re:Depends on who's reading it

schwern on 2004-07-15T14:58:27

Don't remember that split returns a list?

?

Furthermore, ?!

Re:Depends on who's reading it

gav on 2004-07-15T15:04:42

If somebody doesnt't know that split returns a list then they're not a Perl coder and I don't want them touching code.

I don't buy the theory that code should be readable by an average programmer. I code so that a competent programmer can understand it.

Temp variables == clarity... sometimes.

schwern on 2004-07-15T15:07:52

Folks advocate well named temp variables for clarity, similar to well named functions. Its not obvious from glancing at just a split that you're getting a list of keywords. It is obvious from "my @keywords = split ...". However, "foreach my $keyword (split ...)" works just as well.

I have seen this concept abused.

sub foo {
    my $thing = shift;
    my $foo = ...some simple calc...;
    return $foo;
}

Other times folks start out with the long hand version using temp variables. Some people find it easier to think things through that way. Then once its apparent the algorithm is simple you get a little lazy and don't bother compressing it.

Re:Temp variables == clarity... sometimes.

Dom2 on 2004-07-15T19:56:14

Whilst I try to avoid extra variables where I can, I'm not fastidious about it. I will frequently use an extra one if it stops the code leaking over 80 columns...

-Dom

Re:Temp variables == clarity... sometimes.

gav on 2004-07-15T20:08:34

Good point. Lines over 80 columns long are evil.

Re:Temp variables == clarity... sometimes.

Dom2 on 2004-07-17T20:29:20

People often forget that whilst their monitor does more than 80 colunms, their printer usually doesn't.

I know it's not fashionable to print code, but sometimes it's easier to take away and read.

-Dom