chromatic wrote:
> On Monday 26 February 2007 21:20, Michael G Schwern wrote:
>> Case in point... my tests started suddenly warning about "UNIVERSAL::isa
>> called as a function in Class::DBI". After spending a bunch of time trying
>> to figure out what the hell was going on and if Redhat introduced some new
>> warning or something I finally discover that Test::MockObject uses
>> UNIVERSAL::isa() which warns if *anyone* calls UNIVERSAL::isa() as a
>> function. Its nagging *me*, the unrelated 3rd party, about problems in
>> someone else's code that I don't have control over!
>
> No, it's warning you that if Test::MockObject doesn't work, that it's not
> *my*
> problem. It's also telling you exactly *where* that problem is.
If that were so it would only warn when a MockObject is looked at using
UNIVERSAL::isa/can() as a function. That would cover the utility of warning
a user about a possible problem using mocked objects without launching a
UNIVERSAL::isa crusade. As it is it warns on *any* use of UNIVERSAL::isa()
as a function, mocked object or no (although right now UNIVERSAL::isa
appears to not warn at all).
> As much as I'd like to fix the real problem (the belief that enforcing
> structural subtyping makes any sense), the best I can do is try to work
> around the damage while not sweeping it under the rug.
>
> I can't see how ignoring the problem gets it fixed.
While I understand the desire to want to fix other people's code and educate
users, there are many problems with this appraoch. I've outlined them in
detail below but what it boils down to is this...
When I'm programming on a project I want to work on that project. What I do
*not* want to do is have something totally unrelated nagging me to fix
potential bugs in someone else's code. This would be like if you hired some
painters to paint your house and they keep bugging you every five minutes
that the potholes in the street need to be patched up, that my neighbor
really ought to sort his recycling better and my landlord really should have
the water heater inspected. Its distracting and it does nothing to get my
actual work done.
Instead, a course of eduction and contacting the actual authors of the
naughty code is in order. You've certainly started in on the former. As
for the latter, I suggest doing a Google Perl code search for "UNIVERSAL::"
and contacting each of the authors.
Now for the analysis why putting crusader code into otherwise functional
modules is a bad idea:
_Modules are supposed to be modular_ They should avoid universal effects or
poking around in other namespaces without a damned good reason. If I
download a module which does X I do not want it to also launch a Crusade
against other installed code. Stay in your box.
_You're alerting the wrong person_ If there's a problem with a CPAN module
the author of that module should be told, not the users. The users can't do
much but report the problem, which in the case of MockObject vs Class::DBI I
did. And now I twiddle my thumbs waiting for a patch to be applied.
Meanwhile, my tests spew all sorts of warnings that don't indicate any
actual problem and I can't do anything more about.
_You're annoying the user_ By adding on Crusader Code to a module you're
causing annoyance to the users of your module who, as mentioned above, often
can't do much about it.
_You're annoying LOTS of users_ The solution to any particular mistaken use
of UNIVERSAL::isa in a CPAN module is for the author to patch it. Maybe one
user reports the bug and provides a patch. One user, one bug report, one
patch, one author. In order to effect this change you are spewing warnings
at lots and lots of people, many of which can do nothing but report the bug.
_You're conflating two behaviors into one module_ I want to use
Test::MockObject to mock objects. Scanning my code for mistaken uses of
UNIVERSAL::isa is another thing entirely that I may or may not want to do.
By bolting the two together it becomes impossible to do one without the
other. This lowers the utility of MockObject.
_You can break other people's code_ By mucking with other people's code you
potentially *break* that code. By *silently* changing the way isa() works,
or in Eric's case deleting a $SIG{__DIE__} handler, any mistake you make can
now break lots of other totally unrelated code. Silently, unexpectedly and
very difficult to debug because nobody would think that loading MockObject
would have anything to do with Class::DBI (I didn't). As UNIVERSAL::isa and
can have had and currently do have bugs (I can't even get UNIVERSAL::isa to
warn) you take what should be the safe action of loading a module and turn
it into a potential bomb for the whole system.
