Andrew Savige wrote:

--- Ofer Nave wrote:

Here's the POD for my new Parallel::Simple module:



Interface ---------

To me, offering both:

   Parallel::Simple::run()

and:

   Parallel::Simple->run()

just makes the interface bigger -- more for the user to read and
grok -- without any benefit (at least, none I can see). Suggest
you drop the second form (which does not currently work correctly
because the class name is passed as the first parameter and is not
being shifted). Ditto for offering the run() synonym for prun().



I realized last night that it's impossible for me to support both syntaxes with anything better than a total hack, so I'm throwing it out. In fact, I've replaced all occurances of the word 'method' with 'function'. It's no an OO module, it doesn't need method calling syntax. I just tried to put it in initially because 'class methods' seem all the rage now, and I thought I'd just follow the example of those I respect.

I've also now removed any traces of the run() synonym. You're right - why complicate things with no benefit.

Naming. I wonder if your:

       { use_return => 1 },

is the recommended Perl style for named parameters? I thought not
until I noticed HTML::Parser uses this style. Alternatives are
CamelCase style (a la XML::Parser, for example):

       { UseReturn => 1 },

or dash-option style (a la CGI, for example):

       { -use_return => 1 },

I'm damned if I can find a reference clearly stating which one of
these three styles is preferred. Can anyone point me to a reference
on this?



I've seen all three. They're all good, so I'm up for using any one of them. I chose all-lowercase initially to match the identifer naming conventions.

Your example:

  die( Parallel::errplus );

should be written:

  die( Parallel::errplus() );

to avoid bareword error under use strict.



Again, I copied that convention from the greats - in this case, DBI::errstr. In the examples, Tim never includes the parentheses.

What's funny is that I actually like the parentheses, since I strive to avoid any ambiguity, but I left them off here because I was trying to make my first CPAN module as perl-ish as possible - when in Rome and all that. I'll add parentheses back on.

Incidentaly, the above should have read "die( Parallel::Simple::errplus );". I left out the 'Simple::'. Amazing where you find bugs nowadays. :)

Implementation
--------------

Just a couple of micro-optimizations I noticed.

This function synonym:

sub run { prun( @_ ) }

is better implemented as:

sub run { &prun }

This special form of sub call makes current @_ visible to called
subroutine. I suppose the primitive-obsessed might prefer:

*Parallel::Simple::run = \&prun;



After sending my email out, I discovered that realpath() is an alias in the Cwd module and hit the source to see how it's being done. They're using the function aliasing style, which I believe is the fastest ( *alias = \&function ), so I changed my code to that style. Of course, five minutes ago I got rid of the alias entirely (per your suggestion), so this is no longer relevant.

In a couple of places, I noticed:

   /HASH/o

The /o modifier is meaningless here and should be removed.



Ok.

You get the return code here:

   $child_registry{$child}[1] = $? >> 8;

yet miss getting if it died hard from a signal via:

   $? & 127;

Further getting whether it dumped core via:

   $? & 128;

is probably overkill. Not sure how this would affect your interface,
but I've seen cases where a process crashes yet returns a $? >> 8
of zero while $? & 127 is 11 (SIGSEGV).



Seriously?  Is there anywhere I can learn more about this?

-ofer

Reply via email to