http://www.perlmonks.org?node_id=632208


in reply to Practical example of "Is Perl code maintainable"

eyepopslikeamosquito,
If I were writing it, it would probably look something like:
use constant MODE => 2; sub file_mode { my ($file) = @_; return -1 if ! -f $file; return (stat($file))[MODE]; }

I think perl's shortcuts can get in the way of writing maintainable code sometimes. For instance, shift can default to two different variables depending on where in the program it appears. If I were to use shift, I would be sure to explicitly state which variable it was acting on. By avoiding shift, adding additional positional variables doesn't require modifying both sides.

I know that using _ to avoid another stat call is a nice optimization but I would never do it in production code without a comment. I also prefer the use of constants for array indices so that another author can immediately see what is going on.

For the record, I am not a developer and do not write production code so take my comments with a boulder of salt.

Cheers - L~R

Replies are listed 'Best First'.
Re^2: Practical example of "Is Perl code maintainable"
by BrowserUk (Patriarch) on Aug 13, 2007 at 20:14 UTC

    Limbic~Region. I fail to see the ambiguity?

    I am aware of the two different defaults for shift, but if I'm using shift inside a subroutine, and let's face it the vast majority of uses are right up there at the top of the subroutine just below the sub, then there is no ambiguity. So explicitly naming @_ on every use of shift just seems like cargo cult to me.

    If we go down the slippery slope of deeming the implicit use of defaults as bad practice, then we're well on our way to throwing the baby out with the bath water. Today @_. Tomorrow $_.

    The day after that him, her, it, this, that, they ... :)

    As with so many of these 'to avoid confusing the newbie' measures, we're always defending against the lack of knowledge of the newbie maintenance programmer--though I doubt anyone could survive more than a month of coding Perl without becoming familiar with this--but I never see anyone owning up to how they still find it confusing after some minimal period of familiarity with perl.

    And if no one ever is still confused after some minimal period, then who are we protecting?


    Examine what is said, not who speaks -- Silence betokens consent -- Love the truth but pardon error.
    "Science is about questioning the status quo. Questioning authority".
    In the absence of evidence, opinion is indistinguishable from prejudice.
      BrowserUk,
      I fail to see the ambiguity? ... So explicitly naming @_ on every use of shift just seems like cargo cult to me.

      The point I was trying to make "I think perl's shortcuts can get in the way of writing maintainable code sometimes", wasn't based on personal experience as I caveated. When I explain a piece of code to someone who doesn't know perl that well, regardless of how well they program in other languages, I find myself constantly having to explain what is implied but not said. When I explain my own code to someone who does understand perl, they still often complain that my terseness is too much (diotalevi for instance).

      To be clear, I was advocating avoiding shift entirely.

      sub foo { my $this = shift; } sub foo { my ($this, $that) = @_; } sub bar { my ($this) = @_; } sub bar { my ($this, $that) = @_; }
      I find the bar() sub faster to update with less mistakes. I know the mistake is easily found and corrected with warnings and strictures but it helps me to avoid them in the first place.

      I guess working around a bunch of Java programmers who can't understand my code has tainted me. Update: I don't know if I defended my position that well. As I said, I don't have any real world experience with production code. My limited non-professional experience tells me terse implicit code requires another person more time to understand than it would take me to write it explicitly the first time.

      Cheers - L~R

        Update: I don't know if I defended my position that well.

        Actually I though you made your point very well, but there are situations where (IMO) it doesn't make sense to always completely unpack @_.

        For example, in the following sub from my utils library:

        sub rndStr{ join'', @_[ map{ rand @_ } 1 .. shift ] }

        An equivalent might be:

        sub rndStr{ my( $n, @chars ) = @_; return join'', @chars[ map{ int rand @chars } 1 .. $n ]; }

        In use, that would mean the allocation and near immediate destruction of an additional, short-lived array the size of the input list. So when creating a 1GB file of random binary strings:

        local $/; my @chars = map chr, 0 .. 255;; print rndStr 1024, @chars for 1 .. 2**20;;

        it would allocate, destroy and reallocate 256 scalars a million times. For unicode, the list could be much bigger.

        Why? Does the "additional clarity" of naming the parameters for such a short routine really save the follow on programmer such a huge amount of time, to make it worth it?

        From my (mostly non-perl) experience of writing and maintaining production code, I find many proposals for measures and rules intended to "speed the task of the maintenance programmer" completely specious.

        From my (considerable) time spent fulfilling the maintenance programmers role, including a 6-month stint maintaining a huge behemoth of a relational database management system (IBM DB2 1.x), written entirely in COBOL (of which, when I started, I had exactly six weeks (12 hours) of experience from collage about 15 years before), I know that the vast majority of the time is spent

        • understanding and reproducing the bug-report;
        • or understanding the request-for-change;
        • locating the affected files;
        • tracking down where the changes have to be made;
        • working out the implications of those changes and their affects upon the rest of the file(s), subsystems and the overall system;

        than is ever spent actually making the modifications.

        Even when working in a language with which you are almost completely unfamiliar (MicroFocus COBOL is so far removed and extended over the primitive version I learnt at collage in the early '80s, that it is, for all intents and purposes, a completely different language. It has pointers and dynamic memory allocation for dogs sake!), that the time spent understanding the syntax of individual lines of code, or even whole subroutines is very minor when compared to analysing the structure of the surrounding code and how the changes will affect it.

        Indeed, I think that a certain amount of terseness and lack of clarity is a good thing. Anything that makes the follow on programmer stop and think and analyse rather than practicing 'hit&run' maintenance is a good thing in my book. About half of all the maintenance work I did on that rdbms was re-work. Fixing bugs introduced by previous maintenance. In most cases it meant locating and backing out the changes made by earlier 'bug-fixes', and then re-addressing them from scratch.

        And I can hear the "why didn't your test suite detect the introduced bugs" posts being written, but there was a fully automated and very large regression test suite in place. And every maintenance change had to pass through those tests before the change could make it through the system. But with software systems the size and complexity of a rdbms, and given the infinitely variable nature of the inputs (SQL and DB schema's and 'the data', whatever it may be), it takes literally years to evolve your test suites to the point where all possible code paths are exercised.

        And in truth, in such complex systems, with the codebase evolving in parallel to the production deployments (as I was maintaining DB2 1.x. DB2 2.0 was being (re-) written from the ground up in C), they frequently never do exercise the full range of possibilities. It's a simple commercial fact of life.

        So, whilst your POV makes perfect sense in many cases, as with all thou shalts, there will always be exceptions where it simply does not make sense.

        And that is pretty much the basis for all the contrary POV I express here. Blanket application of thou shalts ( and thou shalt nots also), can create as many problems as it hopes to address. Rules of thumb, guidelines and best practices should be quoted often--but with the acknowledgement that there are exceptions and, preferably, with mention of (some of) those exceptions sufficiently regularly to ensure that people are aware that it is okay to make their own judgements when warranted.

        That's why I applaud Perl Best Practices, fear Perl::Critic and reserve the right to argue with and/or ignore both when my faculties of reason and judgement say I should.


        Examine what is said, not who speaks -- Silence betokens consent -- Love the truth but pardon error.
        "Science is about questioning the status quo. Questioning authority".
        In the absence of evidence, opinion is indistinguishable from prejudice.

      In sum: "If we extend this practice to ridiculous extremes, then we end up doing ridiculous things. Therefore, the practice as proposed is certainly ridiculous."

      Although already having acquired a preference for avoiding using shift in particular without arguments (having actually run into code that got moved into a subroutine and thus broken, having seen uglier work-arounds for things like $hash{shift}, and just coming to the conclusion that saving three keystrokes doesn't balance the even minor loss of clarity), I finally fully adopted a "best practice" after a frustrating bought with Test.

      I was trying to fill in some details that were not clearly explained by the Test.pm documentation as to how the arguments to ok() are used. Looking at the source (search for "sub ok") we find a fairly short routine so it should be no problem to figure out how the arguments are used. Searching for @_ or $_[ finds no real uses of arguments (only checks as to whether @_ is empty). So next I had to search for all of the different ways that @_ might be used implicitly. Do you know them all off the top of your head? Are you sure? Personally, I didn't want to perform that mental exercise at that point. Turning up a few "hidden" uses of @_ still left me wondering if I'd found them all.

      So I endeavor to no longer leave such mental land-mines for others (nor for my future self). No, I was not a Perl newbie at the time (Test.pm didn't exist when I was a Perl newbie).

      Now, although I find the small improvement offered by replacing "shift" with "shift @_" to be wholely worthwhile in that particular case, I think the clarity of that subroutine would have been even more helped had shift not been used at all and, even more so, if the argument processing had not been spread over nearly the entire 60 lines of the subroutine. But those are more complex style points that I'm not convinced aren't worth "violating" in some situations so I keep them around as style guidelines to be weighed.

      But I see no even moderate down-side to being explicit with @_, so being explicit with @_ is something that I just always try to do, saving myself from spending any time wondering whether I should leave it off or not. It also saves me from having to worry about using shift by itself in a bareword context and wondering whether @ARGV was really intended. I also enjoy the presense of @ making it that much faster to parse the code.

      You can certainly assume that someone's reasons for having a policy are likely "cargo cult" in the face of no evidence either way. Making a choice to prefer to see other people in such a light is a personal matter. I note that you gave not a single benefit to leaving out @_, so I'll just presume that this is due to a personality flaw on your part, since my experience is that you wouldn't leave out such arguments if you had any. ;)

      - tye        

        ... so I'll just presume that this is due to a personality flaw on your part ...

        Which is exactly why I chose to explore my thoughts and reasoning on the subject with Limbic~Region where I knew I would get a polite, respectful response even if he didn't agree with me.


        Examine what is said, not who speaks -- Silence betokens consent -- Love the truth but pardon error.
        "Science is about questioning the status quo. Questioning authority".
        In the absence of evidence, opinion is indistinguishable from prejudice.