Hi Nadim,
Thank you for all the great suggestions. I really appreciate your honesty,
not to mention the time you took to review the module. I am definitely
going to make most of the suggested changes before submitting to CPAN. In
my research, the modules I saw either didn't quite do what I needed, or
were way too featureful, and therefore cumbersome, for my purposes. That
is, I needed to spawn some independent jobs in parallel, monitor their
output, and make adjustments as these processes were running. Based on
your feedback, I'll try to clarify the description. I'll also add a See
Also section as you mentioned.
I hadn't heard of Perl::Critic, but I am definitely going to give that a
look before proceeding. And, I'll also use Carp, which is definitely the
better solution, although returning undef in most cases was sufficient for
me. I don't think I'll add another pipe, though, for several reasons.
First, it was written to launch jobs that could otherwise be run
independently. The caller might need to terminate a process by sending a
signal, and can also pass any arguments or input at the time the job is
launched. I left this part up to the caller as it's more flexible. The
true value is in being able to monitor the jobs' output. Second, if more
sophisticated IPC / message passing is required, then this is where other
modules such as POE would come in - I don't want to duplicate what they
do.
Can you explain the comment you made here?
> sub get_names # return array of active processes by name
> {
> ...
> my @ret = []; #NKH, Hu!
All in all, I do think the module fills a niche. I'll submit it to CPAN
after making the improvements you and others have suggested. You've given
me some great pointers, and a lot to think about too. I'll revise the
documentation so that, hopefully, the benefits of using this module are
more clear. Thanks again for taking your time to provide this great
feedback.
cheers,
Kevin
<quote who="nadim khemir">
> Hi,
>
> I see the same mistake in this module as in 99% of the modules (including
> mine). Why did you write it, with examples of what was wrong with the
> other
> modules. I said with examples.
>
> The fathers of the CPAN nation were wise. They advised a "See Also"
> section
> which should be very detailed since we never, ahem, write anything without
> researching the subject thoroughly.
>
> Were there other modules you could have enhanced?
>
>
> # causes Illegal seek, but safe to ignore? Euu, well you tell us before
> you
> release.
>
> Did you run Perl::Critic on it?
>
> IMVHO, because formatting issues are so not interesting,
>
> my $self = shift;
> my $name = shift;
>
> is much better written
>
> my ($self, $name) = @_ ;
>
>
> sub get_names # return array of active processes by name
> {
> ...
> my @ret = []; #NKH, Hu!
>
> You write:
> The above example works but does not really demonstrate the full power
> of the module. In a more useful implementation (e.g. using
> Parallel::Jobs) the parent thread may continuously monitor all workers'
> output and provide management such as spawning new workers or sending
> signals to children by their pid. detach() and forget() could be called
> from the run_on_finish() callback of Parallel::Jobs, depending on the
> child's exit status or output.
>
> That paragraph would totally remove any lust I'd have to use the module!
> If
> you have a better example, give it instead for giving a lower usefulness
> one.
> Then you go:
>
> Note: only one pipe is created, with a pair of filehandles for uni-
> directional communication, i.e. from child to parent. Bidirectional
> IPC might be a nice enhancement for this module, or it may be an
> indication that a more robust solution, such as POE, is called for.
>
> Hmm, well, IMO, think again about the usefulness of the module and if you
> should put it on CPAN. Frankly, I am not convinced and as you see above, I
> did
> take some time to look around. Don't get me wrong, The more module authors
> the
> better. Also, the better the documentation, the more users, the better.
>
> Cheers, Nadim.
>
>
>
>
>
>
>
>
>
>
>
