|Think about Loose Coupling|
Silly code reviews and shiftby tilly (Archbishop)
|on Jan 18, 2001 at 13:11 UTC||Need Help??|
This came out of Re: Re: How to pass a variable to function selected from a hash. I was originally going to post there but figured this would be of wider interest.
cat2014 mentioned there that she had been told that shifting off the input to a function was a bad horrible thing to do. I asked her where she heard that, and she said that somewhere she worked would be really hard on you in code reviews if you had done that.
This irritated me. I don't mind code reviews, in fact I think that they are a good thing. But I don't like code reviews that teach people to think of a good chunk of the Perl core and many of the best modules on CPAN as being fundamentally bad code. I particularly don't like that when I would fail their reviews.
So I decided to get a picture of whether or not the standard made sense based on code available for a variety of purposes on a single well-known site. Namely PerlMonks....
My first stop was to my best nodes. I scanned the top 50 looking for ones where I had written a subroutine. I found that 1Y, 2Y, 3Y, and 4Y used shift. And I found that 1N and 2N did not. (I am going to keep a running tally.) Yes indeed, I would do poorly on this code review!
So I decided to try to add to my test all of the Saints other than vroom and me. So I went to the good book and grabbed the top 3 posts from each with subs declared, and looked for whether they had shift. The ones that don't are a grab-bag. Demo subs without any arguments. Ones where it isn't that person's style. Weird funky ones. merlyn contributed one with pop instead which I doubt would go down any better in the reviews. davorg has an amusing one which doesn't really process its own arguments at all...
Here is what I found:
Which makes me curious what other items people out there have encountered in code reviews that didn't particularly make sense to them...