G'day Steve,

Firstly, let me say how appreciative I am that you're willing to go forth and write new modules when the need is shown. Unfortunately, your module leaves me in somewhat of a dilemma. It's close in principle to what I want, and almost, but not quite, satisifies what I need.

My motivation for the module is very specific. I want to make it easier and simplier to do the right thing, than it is to do the wrong thing. Presently, doing the wrong thing is easy:

system("some_command");

This doesn't do any sort of error checking, and so is a great spot for a bug to hide. Using IPC::Run::Simple for the basic case does make it easier:

use IPC::Run::Simple qw(:Fatal);
run("some_command");

However for the next most simple case, it suddenly much longer, and not as simple:

use IPC::Run::Simple qw(:Fatal);
my $exit_value = run(command => "some_command",
                     allowed => [ 0, 1, 5 ]);
[download]

I sill feel it provides a simplier interface for this module to get rid of named arguments entirely:

my $exit_value = run([0, 1, 5], "some_command");

especially as that makes it easy to modify an existing (single-argument) line to allow new exit values.

My other big concern is that run() does not throw exceptions by default, which means the developer has to remember to do extra work in order to do the right thing (either with :Fatal, or by using or die).

It may seem that I'm being incredibly nit-picky, but I'm really trying to make a module that's as fool-proof as can possibly be. I want the default choice to be the right choice even if the developer is ignorant, slack, doesn't read the documentation, and possibly doesn't even understand Perl. Think of your sysadmin archiving files before deleting them. You want that script to die if the archive fails.

Yes, for me "doing the right thing" == "throwing an exception". Damian discusses this extremely well on pages 274-278 of Perl Best Practices.

Having said all that, I really want to avoid the CPAN having two modules that do almost the same thing. Goodness knows we have enough of that already.

I'd very much appreciate further discussion on this topic, and I would love to hear your thoughts.

All the very best,

I really have to buy that book, since it's becoming the shared starting point for so many topics.

I admit I slapped together the API to suit my style, and only incidentally made sure it accomplished your primary goals. Clearly, my preferences don't match yours.

I find it unexpected that a core function would default to throwing exceptions, and it's a major enough difference that I (as a user) would be surprised if a simple wrapper for a core function changed that part of the API. Very few of Perl's existing builtins throw exceptions. Perhaps they should, but in general I am wary of that approach because it means that the called function is deciding what conditions are exceptional, rather than the caller. For some functions, that seems reasonable, but the return value of external processes doesn't seem like something that can be decided without knowing what the program is and why you're calling it.

To play the devil's your advocate _(sorry), it is reasonable to interpret particular return values as exceptional if a required part of the API is listing out exactly what return values are expected.

Which brings me to your second point, that the API should be run([allowed retvals], "command", "arg1", "arg2", ...). I just couldn't bring myself to do it this way, because I find the arguments too nonobvious -- if system() were to take an additional array ref argument at the beginning, what should it mean? Judging from the related modules (IPC::Run, IPC::Cmd), it appears that the first choice of an additional argument is a buffer or buffers to place output into. The inference that it is a set of allowed return values is not straightforward to me -- in fact, at first glance at your sample call, I assumed it was a list of file descriptors to do something special with. (But maybe that's just me.)

The latter is a bigger issue in my mind than the former (whether exceptions are the default.) Once again, if you're in the mindset of "a safe system() replacement that prevents common mistakes", then I think your API is exactly right. But that's not what I expect from a minimalist system() replacement.

If the replacement were called safe_system(), then I could easily agree with your API choices. Or if the module were named IPC::Run::Safe. Perhaps "safe" isn't the even the right word to describe what you're after, though... "sane", maybe?

Hmm. So I guess you can either (1) convince me of the error of my ways; (2) come up with another function name that implies what it's doing better (eg safe_system, but preferably shorter!), in which case I'll happily add it to IPC::Run::Simple; or (3) release your own module with a different name. If you choose #3, I might very well use it, because I occasionally do have need of what you've described -- for pretty much exactly the reasons you've laid out; it's just less common than my desire for a simple system() replacement.

For some functions, that seems reasonable, but the return value of external processes doesn't seem like something that can be decided without knowing what the program is and why you're calling it.

Absolutely correct! However, I think there are some cases we can consider as always being exceptional. Dumping core, being killed by a signal, or failing to execute at all are almost never part of "normal" operation. A program that considers those "normal" is in itself exceptional.

If system() were to take an additional array ref argument at the beginning, what should it mean?

You raise a good point here. I've got a strong motivation to have the easy action be the right action. Adding a set of return values at the start (especially after one has written the code) is easy, but as you've noted it's not particularly obvious. I'd like it to be easy and obvious, if I can. Feedback and thoughts on this in particular would certainly be appreciated.

Once again, if you're in the mindset of "a safe system() replacement that prevents common mistakes", then I think your API is exactly right.

That's very reassuring. Thank-you. ;)

So I guess you can either (1) convince me of the error of my ways; (2) come up with another function name that implies what it's doing better (eg safe_system, but preferably shorter!), in which case I'll happily add it to IPC::Run::Simple; or (3) release your own module with a different name.

We may actually be heading down route #3 here, simply because it gets closer to the "simple-as-can-be" goal for the end-user, and I'm very hesitant to make any "simple" module more complex than it needs to be.

Paul Fenwick
Perl Training Australia