Re: Current Issues with perlipc.pod - should they be fixed?

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.

> I'll comment on some specific items below.
> 
> >> Shouldn't it be using "or" instead of "||" or maybe an if?
> 
> I prefer "or" but I don't terribly care.
> 

I can let it slide. Just noted it.

> >> 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});
>     @signo{@signame} = (1) x @signame;
> 

Actually, @signo should contain the signal number:

	@signame = split(" ", $Config{sig_name});
	%signo = map { $signame[$_] => $_ } (0 .. $#signame);

> >> * unless (kill(0 => $pid) || $!{EPERM}) {
> >>         warn "$pid looks dead";
> >>     }
> >> 
> >> unless and and ||? That's a bit confusing.
> > 
> > I'm sorry you're confused.  Me, I find DeMorgan's
> > Law confusing and therefore avoid its application
> > wherever possible.
> > 
> >    unless I killed it or else I got a permission error
> >        it looks dead
> > 
> > is perfectly clear.
> > 
> > Let's try it your way:
> > 
> >    if (!kill(0 => $pid) && !$!{EPERM}) {
> >        warn "$pid it looks dead";
> >    }
> > 
> > See what a mess you've made?
> 
> I'm torn.  The semantics of a "successful" kill 0 meaning "alive" and
> the use of "$!" vs "! $!" make this ugly both ways.  We're into
> double/triple negative territory.

:-).

> 
> >> my $ALARM_EXCEPTION = "alarm clock restart";
> >>     eval {
> >>         local $SIG{ALRM} = sub { die $ALARM_EXCEPTION };
> >>         alarm 10;
> >>         flock(FH, 2)    # blocking write lock
> >>                         || die "cannot flock: $!";
> >>         alarm 0;
> >>     };
> >>     if ($@ && $@ !~ quotemeta($ALARM_EXCEPTION)) { die }
> >> 
> >> 
> >> Non-lexical filehandle.
> > 
> > Touch noogies.
> 
> I'd rather not touch anyone's noogies, thanks.
> 
> I think it reads fine either as "FH" or "$handle".  If there is a
> general principle of "avoid non-lexical filehandles" then it's worth
> changing.  Short of a consensus (or pumpking) decision, I think it can
> be left alone.  It's not *opening* FH, it's just an example of flock()
> 

OK.

> >> # 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.
> 
> 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: @$_";
>         }
>     }
> 
> Otherwise, the original example as is is not distinguishing between
> failure (1) and can't run (-1), though I don't really know if that is
> important for the purpose of the example.

I don't think it is.

> 
> >>  chdir();    # go home
> >>     my $FIFO = ".signature";
> >> 
> >>     while (1) {
> >>         unless (-p $FIFO) {
> >>             unlink $FIFO;   # discard any failure, will catch later
> >>             require POSIX;  # delayed loading of heavy module
> >>             POSIX::mkfifo($FIFO, 0700)
> >>                                 || die "can't mkfifo $FIFO: $!";
> >>         }
> >> 
> >>         # next line blocks till there's a reader
> >>         open (FIFO, "> $FIFO")  || die "can't open $FIFO: $!";
> >>         print FIFO "John Smith (smith\@host.org)\n", `fortune -s`;
> >>         close(FIFO)             || die "can't close $FIFO: $!";
> >>         sleep 2;                # to avoid dup signals
> >>     }
> >> 
> >> Bareword filehandle,
> > 
> > Cope.  VETO.
> > 
> >> and 2-args open
> > 
> > THIS IS A MYTH!
> > 
> > There is nothing whatsoever wrong with using two-argument
> > open when you have complete control over the content of
> > those strings and you are not setting the encoding in the
> > open itself.
> 
> 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.

> >> 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.
> 

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. 

> >> And here's a multithreaded version.  It's multithreaded in that
> >> like most typical servers, it spawns (fork()s) a slave server to
> >> handle the client request so that the master server can quickly
> >> go back to service a new client.
> >> 
> >> 
> >> The term "multithreaded" is misleading it. It is multi-processed - not
> >> multithreaded, because it does not use threads.
> > 
> > It was how we talked about things when that code was written.  I wrote
> > plenty of perl4 client-server code, and that's what it was.  I believe
> > the Camel and the Cookbook have notes clarifying the original usage.
> > I can dig those out if need be.
> 
> I think it can be modernized with no loss of clarity and with less
> potential for confusion to someone who reads it literally.
> 

+1.

> >> We should no longer mention that modules file.
> > 
> > ACK.
> 
> +1
> 

+1.

> >> Should I send patches to correct all these issues
> 
> Shlomi, I would encourage you to do things in small chunks, carefully
> rebased off the trunk.  The email you wrote to start this thread was
> the right level of granularity.  Some of your past edits were so
> sweeping that it was hard to comprehend the impact.  You were also
> regularly merging trunk into your branch, which made actually
> following your commit series very difficult, and thus hard to comment
> on or apply.

OK, I'll try to wrap my head around git rebase.

Regards and thanks,

	Shlomi Fish

-- 
-----------------------------------------------------------------
Shlomi Fish       http://www.shlomifish.org/
"The Human Hacking Field Guide" - http://shlom.in/hhfg

<rindolf> She's a hot chick. But she smokes.
<go|dfish> She can smoke as long as she's smokin'.

Please reply to list if it's a mailing list post - http://shlom.in/reply .