Beefy Boxes and Bandwidth Generously Provided by pair Networks
Problems? Is your data what you think it is?

Re: Tact and the Monastery

by kschwab (Priest)
on Sep 14, 2002 at 15:02 UTC ( #197868=note: print w/replies, xml ) Need Help??

in reply to Tact and the Monastery


I'm always leery of these "this other guys code was a mess" comments. Certainly it's sometimes (mostly?) true, but "messy" is in the eye of the beholder.

For example, I tend to use lots of temporary variables where they probably aren't needed. I've gotten negative feedback on that. I think, however, that they make the code more maintainable:

# crypt the password my @chars=(a..z,A..Z,0..9,'.','/'); my $salt= $chars[rand(@chars)] . $chars[rand(@chars)]; my $crypted=crypt($password,$salt);
Now, the @chars and $salt variables aren't really needed, and I could have done something like:
my $crypted = crypt( $password, join( '', ('.', '/', 0..9, 'A'..'Z', 'a'..'z')[rand 64, rand 64]));
which is more compact, but less readable, IMO.

I've also gotten negative feedback on code reviews for overly defensive programming, which again, adds lines of code and generally makes for more "mess". ( additional eval blocks, lots of defined() calls, etc).

Replies are listed 'Best First'.
•Re: Re: Tact and the Monastery
by merlyn (Sage) on Sep 15, 2002 at 00:34 UTC
    My rule is that a temporary variable is fine if it can be related to the application in some way, and not just an artifact of optimization. One way to tell is if you can give it a truly meaningful name. If you can't, see if you can eliminate it.

    -- Randal L. Schwartz, Perl hacker

      My rule is that a temporary variable is fine if it can be related to the application in some way, and not just an artifact of optimization.
      One way to tell is if you can give it a truly meaningful name. If you can't, see if you can eliminate it.

      There is sense & beauty in that. But it does require a high degree of Perl knowledge.

      Corollaries can be derived.

      Temporary variables should be named so that it is obvious that they are temporaries.

      Temporary variables should be scoped to advertise their ephemeral nature.

      For those who can't readily parse dense Perl temporary variables are like the x modifier to m//.

      a temporary variable is fine if it can be related to the application in some
      way, ... One way to tell is if you can give it a truly meaningful name. If you can't,
      see if you can eliminate it.

      From early on I have been a believer in good names. The above as inspired me to
      raise the level of quality I try to achieve in naming. A good name should
      clarify obscure/advanced/complex/efficient 1 code by expressing what the variable
      should be.

      It is going to be tough to break the habit of using result for return values.

      1 Choose one. Define yourself.

        Here is a example of trying to learn from our betters and getting confused. The node Randal does it again refers to an article by Randal Schwartz.

        I found this snippet of code that uses a variable name $old_flag.
        sub wanted { return unless -f $_; my $old_flag = -M $_ > 180 ? 'old' : 'new'; $ages{$File::Find::dir}{$old_flag}++; }
        My confusion arises because I would have expected $old_flag to hold a true/false value. Shouldn't this variable be named something like $age_status. This would make it "sound" better to say : $age_status is new or old.

        How should this be handled?

Re^2: Tact and the Monastery
by Aristotle (Chancellor) on Sep 15, 2002 at 07:55 UTC
    I find your first example unnecessarily noisy, while the second is too golfish. Balance is key. Mine would look like so:
    my @chars = ('a' .. 'z', 'A' .. 'Z', 0 .. 9, '.', '/'); my $crypted = crypt $password, $chars[rand @chars].$chars[rand @chars] +;

    That way I can't make a mistake in the size of the list vs the constant passed to rand. Nor will I have to figure out why I used 64 there if I read the source again in a year. It also lets me use concatenation vs the noisy and distracting join/slice method.

    $salt on the other hand is superfluous - that value is trivially calculated on the fly.

    Makeshifts last the longest.

Re^2: Tact and the Monastery
by Flexx (Pilgrim) on Sep 14, 2002 at 22:36 UTC

    OTOH... ;)

    I personally think the second of your examples really is more readable. But maybe I'm just a bit nuts.

    Myself, I use as few temporary (synthetic) variables as possible, I think they often make things harder to understand. Even if I immediately get the idea what @chars is, I would glance back on every occurance of it, to make sure what your idea of "characters" really is (with or without underscore, hypen, etc.). If the code spreads out, things become even worse...

    In your second example, it's right there.

    Which brings us back to the topic of this thread. While I am sure that there are some style ideas, any good Perlmonk would propagate, some other things really are a matter of taste.

      I don't like the comma-prepending style. You can easily add a new line at the bottom that way, but not at the top. It might be fine for languages that are inflexible about their commata, but Perl is not - you can append one to the last value with no harm. My formatting looks like this:
      my %hash = ( one => 1, two => 2, three => 3, );
      Note the "errant" comma after the last value. Separating the assignment and closing bracket on their own lines adds more convenience. This way, I can add a hash key anywhere - even at the bottom or the top - without having to edit any other line than the one I'm on.

      Makeshifts last the longest.

Re: Re: Tact and the Monastery
by helgi (Hermit) on Sep 17, 2002 at 11:46 UTC
    I'm with you on this one kschwab.

    I certainly try not to use too many temporary, intermediate variables, but I do if they add to readability for a future maintainer of the code.

    My basic criteria for using a temporary variable is this:
    If I can think up a meaningful, transparent name for it, I can use it.

    For example, in your case, @chars and $salt are both meaningfully named, thus allowed. What I would not allow would be @array and $foo for the same variables.

    Helgi Briem

Re: Re: Tact and the Monastery
by talexb (Canon) on Sep 20, 2002 at 17:01 UTC
    I would probably change that to look like
    # crypt the password my $crypted; { my @chars=(a..z,A..Z,0..9,'.','/'); my $salt= $chars[rand(@chars)] . $chars[rand(@chars)]; $crypted=crypt($password,$salt); }
    so that the temporary variables (@chars and $salt) disappear after the closure ends. And I agree with the suggestion lower down: you could lose $salt and incorporate the salting in the call to crypt.

    --t. alex
    but my friends call me T.

      That's not a closure. :-) It's just a naked block.

      Makeshifts last the longest.

        I have to disagree: because I have declared a variable inside that block, it becomes a closure. When the closure ends, the variables cease to exist.

        This is a Good Idea because it means the variables a) aren't sitting around waiting to Mess Things Up later on when the next piece of code uses variables with the same name and b) aren't using up memory that's no longer needed.

        Hence, my downvote.

        --t. alex
        but my friends call me T.

        Update:And sometimes I'm an idiot.

        You're absolutely right, that's not a closure. I was getting closure and scope mixed up. A scoped variable does disappear when it goes out of scope. A variable defined within a closure continues to exist for as long as the closure exists.

        My apologies.

Log In?

What's my password?
Create A New User
Node Status?
node history
Node Type: note [id://197868]
and all is quiet...

How do I use this? | Other CB clients
Other Users?
Others rifling through the Monastery: (3)
As of 2018-03-22 02:51 GMT
Find Nodes?
    Voting Booth?
    When I think of a mole I think of:

    Results (272 votes). Check out past polls.