Beefy Boxes and Bandwidth Generously Provided by pair Networks
We don't bite newbies here... much
 
PerlMonks  

Re^5: Is this DBM::Deep behavior, or something with tie/bless? (ref)

by tye (Sage)
on Feb 09, 2008 at 06:49 UTC ( [id://667145]=note: print w/replies, xml ) Need Help??


in reply to Re^4: Is this DBM::Deep behavior, or something with tie/bless? (normal)
in thread Is this DBM::Deep behavior, or something with tie/bless?

Sounds like Data::Compare is broken. A quick peek at the source turned up exactly the type of broken code that I expected to see:

if(ref($requires) ne 'ARRAY') {

So just fix Data::Compare. The fixes would probably be quite simple. The best fix is to change such things to:

if( ! eval { @$requires; 1 } ) { # Can't be used as an array re +f

It is fine to use ref as a Boolean test. Any other uses of ref I simply can't recommend.

Note that the above trick doesn't work for CODE references so you have to resort to one of the second-best methods. I'd use the following:

*isa= UNIVERSAL::isa; #... if( isa( $ref, "CODE" ) ) {

Note that this test can fail in the case of overloaded objects that want to pretend to be CODE references but that didn't bother to push @ISA, "CODE"; in order to declare this intention (which seems a perfectly reasonable restriction to me). chromatic would surely cringe and moan upon seeing such code because surely $ref->isa("CODE") is what should be used (except, of course, that it is likely to die in many cases). eval { $ref->isa("CODE") } might be a possible alternative but I thought it had its own drawbacks even though I can't recall what they were. My preferred method can also produce false positives if somebody intentionally lies via push @ISA, "CODE"; which I also consider to be a perfectly reasonable feature (which can even be useful when writing unit tests, for example).

Looking at the code further, I see one spot that would be somewhat complex to fix because it assumes that a reference can only be of one type, which is not the case. But it would also simplify other parts of the code, because there would not have to be special code for looking under the covers of blessed references.

Actually, perhaps it would be best to leave the plug-in handling alone, despite the flawed assumptions present there. Replacing the flawed assumptions with proper handling when considering handlers for specific classes of objects would be quite complex and you don't need to fix plug-in support in order to fix the basic flaw in the module; thus making it work fine on DBM::Deep results.

Then the module becomes quite easy to fix (and the fixes still simplify some parts). The best route would probably be to write the following tiny helpers:

sub isArray { eval { @{$_[0]}; 1 } } sub isHash { eval { %{$_[0]}; 1 } } sub isScalar { eval { ${$_[0]}; 1 } } sub isCode { UNIVERSAL::isa( $_[0], "CODE" ) }

And then the less-tiny helper:

sub getCommonRefType { my( $ref1, $ref2 )= @_; return "ARRAY" if isArray($ref1) && isArray($ref2); return "HASH" if isHash($ref1) && isHash($ref2); return "SCALAR" if isScalar($ref1) && isScalar($ref2); return ""; }

- tye        

Replies are listed 'Best First'.
Re^6: Is this DBM::Deep behavior, or something with tie/bless? (ref)
by ysth (Canon) on Feb 12, 2008 at 16:09 UTC
    So just fix Data::Compare. The fixes would probably be quite simple. The best fix is to change such things to:
    if( ! eval { @$requires; 1 } ) { # Can't be used as an array ref
    It is fine to use ref as a Boolean test. Any other uses of ref I simply can't recommend.

    Note that the above trick doesn't work for CODE references so you have to resort to one of the second-best methods. I'd use the following:

    Wouldn't that give a useless use warning? I would suggest this instead, which also works for CODE.
    if( ! eval { \@$requires } ) { # Can't be used as an array ref
    Pre-perl 5.10, either has a problem with arrayrefs being able to be dereferenced as pseudo-hashes.

      Yes, I have an open bug against Data::Diver for issuing warnings about pseudo-hashes due to this.

      You are right about the other warnings as well. Unfortunately, your alternative has other problems:

      my $foo; if( eval { \@$foo } ) { print $foo, $/; } __END__ ARRAY(0x34d10)

      In the case of CODE ref testing, there is a different problem:

      my $foo= "not_a_code_reference"; print "oops!\n" if eval { \&$foo };

      Of course, in some situations, one could consider the latter a feature. But mostly I think it would be unwanted.

      It is very sad that Perl still doesn't provide decent tools for determine the data type(s) of a reference. It is no wonder nobody gets this right.

      - tye        

Re^6: Is this DBM::Deep behavior, or something with tie/bless? (ref)
by dragonchild (Archbishop) on Feb 12, 2008 at 16:57 UTC
    A few thoughts:
    • Those isX() methods belong in Scalar::Util (or something similar). I have similar snippets in DBM::Deep itself for the exact same reason.
    • You need to localize $SIG{__DIE__} within those evals. In doing this for DBM::Deep, I found Test::More does things with die handlers that get tripped up with this.

    My criteria for good software:
    1. Does it work?
    2. Can someone else come in, make a change, and be reasonably certain no bugs were introduced?

      Rather, $SIG{__DIE__} handlers need to detect that eval is in effect ($^S -- even though it still isn't perfect). $SIG{__DIE__} is (by far) the 'worse' magic and so needs to take the burden for "playing nice". "Simple" use of eval doesn't need to be made even more complicated. local( $@ ); is another safety measure that is missing from the simple examples. Quite the mess.

      - tye        

Log In?
Username:
Password:

What's my password?
Create A New User
Domain Nodelet?
Node Status?
node history
Node Type: note [id://667145]
help
Chatterbox?
and the web crawler heard nothing...

How do I use this?Last hourOther CB clients
Other Users?
Others avoiding work at the Monastery: (7)
As of 2024-03-28 15:10 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    No recent polls found