On Fri, Dec 03, 2010 at 07:46:09PM +0200, Shlomi Fish wrote:
> On Friday 03 December 2010 19:30:14 David Golden wrote:
> > Let me say to all in the thread so far that my general reaction to the
> > original list was "hmm. seems fairly reasonable".
> >
> > However, I think that if we take the emotion out of the thread (and
> > terms like "badly-written") and recognize that we *are* having a style
> > debate, then we have a few options:
> >
> > (a) original style shouldn't be changed
> > (b) anyone motivated to submit style patches should have them applied
> > (c) we look to a style guide (or general style principles) for guidance
> >
> > The problem I have with (a) and (b) is that it leaves us, ultimately
> > with one particular person's view of "good" style the the potential
> > for very diverse styles across Pod. TIMTOWTDI, but consistency isn't
> > a bad thing, either. (The new acronym hasn't stuck for me yet)
> >
> > I don't really want a hard style guide for documentation -- we've had
> > that debate before, I think. But I wouldn't be opposed to some
> > general principles so we don't have arbitrary style debates like this
> > over and over again.
> >
> > For me, one of the goals that any Pod example over a few lines should
> > have is being copy-paste ready. We know that newbies do that. Heck,
> > even I do it from time to time for obscure stuff I don't use everyday.
> > I copy from CPAN synopses all the time.
> >
> > By way of example (with reference to some of the particular items
> > under consideration) I don't really care about C<||> vs C<or>. Yes, I
> > favor one, but there's no harm done using the other. On the other
> > hand, I really would prefer to see more use of lexicals instead of
> > globals in any snippet that I think people might paste into real code.
> > (FWIW, I often write presentations "non-strict" and omit "my", so I'm
> > not a zealot about it, but that's to maximize presentation font size).
> >
> > So I'd support a general principle like "avoid globals for any example
> > of more than a couple lines".
> >
> > If we get some principles straight, we can be more productive with
> > fewer bruised feelings.
> >
>
> 1+. Sorry for saying "badly-written code". I didn't mean tchrist's code was
> badly written just that the code that some Perl beginners show us is badly
> written, non-idiomatic and/or non-modern, as it was inspired by out-of-date
> books, tutorials, and other resourcs. I meant we should not demonstrate such
> bad coding patterns that we feel are not desirable, by putting them inside the
> core documentation.
Let's get a few things straight.
First, we are talking here about Perl documentation. It's main purpose is
to explain Perl, every nook and cranny. It's not intended as a learning
Perl book. It's a reference guide. Yes, you can learn Perl that way (I
did), but that's not its main purpose. The documenation takes one thing
at a time, discusses it, and, ideally, has a short example showing the
discussed construct, and as little as anything else.
Second, "badly written", and "non idiomatic" are very subjective terms.
Unless you can clearly indicate *why* something is badly written or
"non idiomatic", I rather don't see such labels used. That counts for
the labels "bad coding pattern" and "not desirabe" as well. Larry has
said repeatedly "it's ok to talk baby-Perl".
Third, the term "modern Perl" makes me twitch in a bad way. I don't
understand what it is, or why 2010 is the year to jump on this
bandwagon. (I wonder which piper to follow in 2011).
Fourth, if documentation is out of date, it's not just the code examples
that need to be updated - whatever it is the example show needs to be
updated. (Otherwise, it wouldn't be outdated). But note that unless the
feature has actually been removed from Perl, its documentation should not
disappear.
> > >> foreach $name (split(" ", $Config{sig_name})) {
> > >> $signo{$name} = $i;
> > >> $signame[$i] = $name;
> > >> $i++;
> > >> }
> > >
> > > The tone and style of the Perl Documentation as set by Larry and myself
> > > is
> >
> > That's an appeal to authority type argument, which I don't like. The
> > tone and style were set when globals were more common. I think this
> > is open for reasonable people to debate. Adding "my" costs nothing in
> > obscuring the clarity of the point. Frankly, I think the whole thing
> > could be written more idiomatically. Incrementing $i is just icky.
> > E.g.:
> >
> > @signame = split(" ", $Config{sig_name});
> > @sig...@signame} = (1) x @signame;
> >
>
> Actually, @signo should contain the signal number:
>
> @signame = split(" ", $Config{sig_name});
> %signo = map { $signame[$_] => $_ } (0 .. $#signame);
If we really want to aim at newbies, do we really want to change a simple
loop into a something tricky that tripped up one experienced Perl person,
and needs to be written with a map and the $# sigil?
I prefer the original. In normal code, I would write 'my $name', but there's
only a limited with to play with if you want to resulting manual page to
show in an 80 character term; I don't mind the 'my' not being there. The
example isn't about foreach syntax.
> > >> # system return val is backwards, so && not ||
> > >> #
> > >> $ENV{PATH} .= ":/etc:/usr/etc";
> > >> if ( system("mknod", $path, "p")
> > >> && system("mkfifo", $path) )
> > >> {
> > >> die "mk{nod,fifo} $path failed";
> > >> }
> > >>
> > >>
> > >> We probably want local $ENV{PATH} here, and can't we expect the
> > >> mkfifo system call to work globally already?
> > >
> > > No, we most certainly cannot expect that! I didn't put that code in
> > > there out of imagined problems. I put it in there because if I didn't,
> > > it failed on some of systems I tested it on. Why? Because you cannot
> > > count on the user to have those in his path, that's why.
> >
> > I agree that $ENV{PATH} doesn't need to be localized for this example.
> > This is an example of "find it elsewhere" and messing with $ENV{PATH}
> > is just a reminder of that. Is it important for this tutorial to hint
> > at where it might be? I don't know.
How many systems are there were 1) mknod and mkfifo are in either /etc/
or /usr/etc/, and 2) the directory they're in aren't in the standard root
PATH? Maybe it's better to take out the $ENV{PATH} line out, and replace
it with something like:
# Make sure 'mknod' and 'mkfifo' are in your PATH.
which explains the $ENV{PATH} line without tying itself to a particular OS.
> >
> > Is it likely for "mknod" or "mkfifo" to fail if they are indeed
> > present in the path? If not, the point might be clearer with IPC::Cmd
> > and can_run:
> >
> > use IPC::Cmd;
> > for ( ["mknod", $path, "p"], ["mkfifo", $path] ) {
> > if (can_run($_->[0])) {
> > system(@$_) and die "Command failed: @$_";
> > }
> > }
I don't find this clearer at all. It requires knowledge of IPC::Cmd, and
untwisting the C<< for ( ["mknod", $path, "p"], ["mkfifo", $path] ) >> line.
Now, one may not understand the original right away, but at least the syntax
doesn't bog one down in understanding - which, IMO, the above does.
[ Another example with bare filehandles ]
> > Same argument on my part as before about principles, but stronger. If
> > we're actually showing an "open" call, I'd much prefer to have
> > three-arg open to a lexical handle. Yes, a two-arg call is safe in
> > this circumstance. But no clarity is lost with a three-arg open,
> > either. Newbies crib documentation and/or adopt the styles they see
> > there. I'd rather they see the "always safe" version instead of the
> > "safe this time" version. Certainly, I understand that Tom disagrees,
> > which is why I'd like an agreed set of guidelines so it isn't just
> > individual style debate yet again.
> >
>
> +1.
I agree with Tom.
Also, newbies *will* encounter bare filehandles once they come out of
kindergarten. No need to shield newbies from them in the core documentation.
> > >> use warnings instead of "-w".
> > >
> > > ACK
> >
> > +1
> >
> > >> my ($remote, $port, $iaddr, $paddr, $proto, $line);
> > >>
> > >> $remote = shift || "localhost";
> > >> $port = shift || 2345; # random port
> > >> if ($port =~ /\D/) { $port = getservbyname($port, "tcp") }
> > >> die "No port" unless $port;
> > >> $iaddr = inet_aton($remote) || die "no host: $remote";
> > >>
> > >> we should declare the variables at first assignment instead of all in
> > >> one place.
> > >
> > > What? Three lines away is too far? I think not!
> >
> > I think it's fine on one line. The 'extra' line vs inline 'my' is
> > trivial in an example of that length.
> >
The only problem I have with this example is that the
'|| die "no host: $remote";' wraps into the 81 column after formatting.
A C<< s/ = /= /g >> would solve this, while keeping the vertical
lining up of the operators (some || don't line up, which is someone
disturbing, but not something I'd change for the sake of changing).
>
> OK.
>
> > >> Shouldn't the example use IO::Socket and friends?
> > >
> > > No, do not Bowdlerize working code.
> >
> > Unless there is good reason why one should use IO::Socket, I don't see
> > a reason to switch. If IO::Socket made the example simpler, then it
> > might be worth considering.
> >
>
> Well, we no longer recommend people to use the lower-level modules instead of
> IO::Socket , so it may again be a bad example and I've already started
> converting the code to IO::Socket in my branch.
Instead of changing code to use IO::Socket, why not *add* an example that
uses it? Or add such an example in IO::Socket, with perlipc having a pointer
to it?
Abigail
