comment on

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!

My second stop was to Best Nodes, what about the top 10 posts here of all time? Well 5Y used shift, eurdil had a couple of dishonourable mentions for coding style. And nobody else declared any subs.

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:

merlyn shifted in 6Y, 7Y and didn't in 3N.
chromatic shifted in 8Y, 9Y and didn't in 4N.
Ovid shifted in 10Y, 11Y and didn't in 5N. (And wrote an impressive number of tomes, but I digress.)
turnstep shifted in 12Y and 13Y and didn't in 6N.
btrott clearly has strong feelings, he shifted in 14Y, 15Y and 16Y.
Adam starts the tide turning. He did not shift in 6N, 7N, and 8N. Adam can ask cat2014 if he wants a job that suits him...
jcwren shifts in 17Y but not in 9N and 10N.
tye shifts in 18Y but not in 11N and 12N.
Fastolfe shifts in 19Y but not in 13N and 14N.
davorg shifts in 20Y but not in 15N and 16N.
lhoward rounds out our saints by shifting in 21Y and 22Y but not in 17N.

So what do I conclude? Well I took a well-known Perl site, and took a sample of what are supposed to be its best posts. Of the ones which have functions, most used shift to process arguments at some point. So now I can with confidence tell cat2014 that she can go back to whoever did her code review and tell them that they failed me, but I find the roster of my very good fellow failures to be very comforting indeed!

Which makes me curious what other items people out there have encountered in code reviews that didn't particularly make sense to them...

In reply to Silly code reviews and shift by tilly

Are you posting in the right place? Check out Where do I post X? to know for sure.
Posts may use any of the Perl Monks Approved HTML tags. Currently these include the following:
<code> <a> <b> <big> <blockquote> <br /> <dd> <dl> <dt> <em> <font> <h1> <h2> <h3> <h4> <h5> <h6> <hr /> <i> <li> <nbsp> <ol> <p> <small> <strike> <strong> <sub> <sup> <table> <td> <th> <tr> <tt> <u> <ul>
Snippets of code should be wrapped in <code> tags not <pre> tags. In fact, <pre> tags should generally be avoided. If they must be used, extreme care should be taken to ensure that their contents do not have long lines (<70 chars), in order to prevent horizontal scrolling (and possible janitor intervention).
Want more info? How to link or How to display code and escape characters are good places to start.


Come for the quick hacks, stay for the epiphanies.
	PerlMonks