|No such thing as a small change|
Re^3: Modules which improve system() ?by sfink (Deacon)
|on Jun 27, 2006 at 07:25 UTC||Need Help??|
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.
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.