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

[romandas]

The reason for 'untie'ing the array is related to my program using Data::Compare to compare data structures. The DBM::Deep database stores a hash of arrays (which have a hash as an element, etc). When my program runs, it retrieves the old info from the database to compare it with the new live information.

However, Data::Compare fails when comparing the two arrays (old and new info) because the old array is tied to the DBM, effectively comparing data structures that aren't alike (my logical/mental representation notwithstanding) since DBM::Deep hashes and arrays are objects, I think, as opposed to the regular hash/array structure of the new information in memory.

The two solutions I thought of (assigning the database array to a temp variable; assigning the new information to a temp spot in the database) both do not work (for differing values of 'do not work') due to the variables getting tied to the database.

I could potentially write a plugin for Data::Compare to handle this, but from what I can see that is way beyond my current ability.

I would post my code, but haven't finished writing it yet. :) The "variable reuse" problem (I believe) is just an artifact of my test code above; I don't think it'll exist in my actual code.

[tye]

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 "";
}

[ysth]

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.

[tye]

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.

[dragonchild]

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.

[tye]

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.

[dragonchild]

I believe that you and I have spoken via email about this topic. I offered to support such a plugin if you would write the initial code and tests for it then hand it over to me. You never replied as to whether or not you would be willing to do so. Just throw some code together and hand it off - not too hard.

[romandas]

You're correct; we have spoken in regards to this.

I did reply to you that I would be willing to write up a plugin, if I felt confident I could do it. But, as my post above stated: "I could potentially write a plugin for Data::Compare to handle this, but from what I can see that is way beyond my current ability." Trust me, I am more than willing to help out in this regard if I can. As for writing up a test for DBM::Deep, I'm still trying to puzzle out exactly how tests work.

All beauty and enthusiasm with no substance at this point, I'm afraid. :)

[dragonchild]

The following is a failing test that demonstrates the problem. I would go ahead and extend it further to additional examples. Then, you now have something to write your plugin against.

use Test::More tests => 1;

use DBM::Deep;
use Data::Compare;

use File::Temp qw( tempfile );

my $filename = tempfile();

my $db = DBM::Deep->new( file => $filename );
$db->{foo} = [ 1 .. 3 ];

my $compare = {
    foo => [ 1 .. 3 ],
};

ok( Compare( $db, $compare ), "The structures are the same" );

Now, you also have an excellent example plugin with [src://Data::Compare::Plugins::Scalar::Properties]. You could copy that module, rename it, and change sp_scalar_compare to dbm_scalar_compare and change its logic a bit. At that point, you can run your test and see if anything passes. At that point, you can hand it to me and we can work with something. Remember - the hardest part is the blank page. Editing is sooo much easier than creating.