develooper Front page | perl.perl5.porters | Postings from February 2015

Re: [perl #123898] [PATCH] Modernize the second part of perldsc.pod.

Thread Previous | Thread Next
Ricardo Signes
February 26, 2015 03:17
Re: [perl #123898] [PATCH] Modernize the second part of perldsc.pod.
Message ID:
* Shlomi Fish via RT <> [2015-02-25T04:27:55]
> On Sat Feb 21 14:34:24 2015, aristotle wrote:
> > Uh, every iteration of the inner loop is spared the lookup of $AoA[$i].
> > 
> > However, while I *would* perform this refactoring on real code, I find
> > it a change for the worse in this instance in that it conceals the
> > correspondence between the structure of the code and the data structure.
> I see - well, I'll wait for a consensus or veto here.

I am with Aristotle.  While I'd make the change in product code, I think the
documentation is better as written for the purposes of demonstration.

> >     + LINES:
> >       while ( <> ) {
> >     -     next unless s/^(.*?):\s*//;
> >     +     next LINES unless s/^(.*?):\s*//;
> > 
> > (Repeat some two dozen times.)
> > 
> > I cannot find a single case where this helps comprehension. It just adds
> > noise. Every single time.
> I added the LINES label per the best practice of
> (which also appears in the book Perl Best Practices). If there's a consensus
> or a veto that it shouldn't be added, then I'll remove it in the redone
> patch.

Here, I'm not with Aristotle except in the practical outcome.  I nearly always
use this pattern, as I find it practical when doing later refactoring (but not
because of skimmability).  On the other hand, I would not change the
documentation.  I think it's clearer without the label.  There is less to take
in without it.

> > The added `my`s are fine, but consider the “after” version. If you have
> > just declared @tmp inside the loop body to hold the return values of
> > a function call, why then flatten it in an anonymous array constructor,
> > which makes a copy of its contents? Just take a reference to it:
> > 
> >     for my $i ( 1 .. 10 ) {
> >         my @tmp = somefunc($i);
> >         $AoA[$i] = \@tmp;
> >     }
> Code like that (= declaring a lexical aggregate variable and storing a
> reference to it in a container variable) is exactly what the text that I
> removed from the previous patchset to perldsc.pod warned against doing (and I
> thought such a warning was no longer necessary in this day and age). So what
> is the verdict regarding doing it?

I had a flip through the history on that file and I couldn't find what you're
referring to — but to be fair, I gave up pretty fast.  If we know that @tmp is
quite local to the loop, which we do, I'd do what Aristotle recommends.

Is your previous change relevant to that?  If so, could you point it out to me?


Thread Previous | Thread Next Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at | Group listing | About