Data::Dumper woes

[Piers Cawley]

I've discovered some problems with Data::Dumper associated with the
C<$Data::Dumper::Toaster> and C<$Data::Dumper::Freezer> variables.
For a start, the functionality that they are used for isn't actually
tested, which isn't good. And, rather more worryingly, they're
inconsistent.

Luckily, writing a test to expose the inconsistencies was trivial (Kudos
to Schwern), and fixing things so that they were resolved in a way
that suits me (and who else do I care about?), was also
trivial. However, before I send in the patch, some questions:

In the stock Data::Dumper::Dumpxs, if a 'Freezer' method dies, we
'downgrade' that to a warning. In the perl implementation, it just
dies. I can see arguments for allowing both the 'warning' version and
the 'just dies' version, though I think the 'just dies' option is the
better of the two (die early, die often...). However, it shouldn't be
hard to make it conditional on a variable. But what to call the
variable? My current favourite is C<$Data::Dumper::Dieonfailure>.

So, should I make this conditional on a variable? If so, what do I
call the variable?

The second inconsistency concerns what is done with the result of the
call to the Freezer method. Here's what the XS version does (recast in
Perl for clarity):

my $newval = eval { $val->$freezer };
if ($@) {
warn "WARNING(Freezer method call failed): $@";
}
else {
$val = $newval;
}

And the perl version:

$val->$freezer if UNIVERSAL::can( $val, $freezer );

What I'd like to see is:

$val = $val->freezer if eval { $val->can($freezer) };

(Yes, I *do* have classes that override 'can'...)

But I'm not entirely sure how to implement that in XS code. Thoughts?

--
Piers

"It is a truth universally acknowledged that a language in
possession of a rich syntax must be in need of a rewrite."
-- Jane Austen?

[h...@crypt.org]

Piers Cawley <pdca...@bofh.org.uk> wrote:
:In the stock Data::Dumper::Dumpxs, if a 'Freezer' method dies, we

:'downgrade' that to a warning. In the perl implementation, it just
:dies. I can see arguments for allowing both the 'warning' version and
:the 'just dies' version, though I think the 'just dies' option is the
:better of the two (die early, die often...). However, it shouldn't be
:hard to make it conditional on a variable. But what to call the
:variable? My current favourite is C<$Data::Dumper::Dieonfailure>.
:
:So, should I make this conditional on a variable? If so, what do I
:call the variable?

I suspect that the majority of users have the XS version (it is the
default where buildable, right?) so we should probably preserve that
as the default behaviour. I'd suggest $Fatal, and (as in the recent
Storable discussion) allow boolean or coderef.

:The second inconsistency concerns what is done with the result of the

:call to the Freezer method. Here's what the XS version does (recast in
:Perl for clarity):
:
: my $newval = eval { $val->$freezer };
: if ($@) {
: warn "WARNING(Freezer method call failed): $@";
: }
: else {
: $val = $newval;
: }
:
:And the perl version:
:
: $val->$freezer if UNIVERSAL::can( $val, $freezer );
:
:What I'd like to see is:
:
: $val = $val->freezer if eval { $val->can($freezer) };
:
:(Yes, I *do* have classes that override 'can'...)

Trouble is, $val->can fails if $val doesn't look enough like an object,
and I've found that when hit by this, UNIVERSAL::can springs to mind
more quickly than eval {}. I'd like to see $x->can always succeed to
help avoid that trap, at least as far as an experiment to see what
breaks.

For now, I'd suggest something more like:
$val = $val->freezer if blessed($val) && $val->can($freezer);
... since a blessed($val) check is far easier (and quicker) in XS
than an eval.

Hugo

[Piers Cawley]

h...@crypt.org writes:

> Piers Cawley <pdca...@bofh.org.uk> wrote:
> :In the stock Data::Dumper::Dumpxs, if a 'Freezer' method dies, we
> :'downgrade' that to a warning. In the perl implementation, it just
> :dies. I can see arguments for allowing both the 'warning' version and
> :the 'just dies' version, though I think the 'just dies' option is the
> :better of the two (die early, die often...). However, it shouldn't be
> :hard to make it conditional on a variable. But what to call the
> :variable? My current favourite is C<$Data::Dumper::Dieonfailure>.
> :
> :So, should I make this conditional on a variable? If so, what do I
> :call the variable?
>
> I suspect that the majority of users have the XS version (it is the
> default where buildable, right?) so we should probably preserve that
> as the default behaviour. I'd suggest $Fatal, and (as in the recent
> Storable discussion) allow boolean or coderef.

It'll already dump a coderef, but only using the Dumpperl version (and
I haven't a *clue* how to go about making it handle a coderef within
the XS version.)

>
> :The second inconsistency concerns what is done with the result of the
> :call to the Freezer method. Here's what the XS version does (recast in
> :Perl for clarity):
> :
> : my $newval = eval { $val->$freezer };
> : if ($@) {
> : warn "WARNING(Freezer method call failed): $@";
> : }
> : else {
> : $val = $newval;
> : }
> :
> :And the perl version:
> :
> : $val->$freezer if UNIVERSAL::can( $val, $freezer );
> :
> :What I'd like to see is:
> :
> : $val = $val->freezer if eval { $val->can($freezer) };
> :
> :(Yes, I *do* have classes that override 'can'...)
>
> Trouble is, $val->can fails if $val doesn't look enough like an object,
> and I've found that when hit by this, UNIVERSAL::can springs to mind
> more quickly than eval {}. I'd like to see $x->can always succeed to
> help avoid that trap, at least as far as an experiment to see what
> breaks.

I've found myself wishing for that on many occasions. The reason being
that I'm the kind of nutter who goes 'round overriding 'can' and 'isa'
in some classes, which can cause confusion when people keep falling
back to UNIVERSAL::isa

> For now, I'd suggest something more like:
> $val = $val->freezer if blessed($val) && $val->can($freezer);
> .. since a blessed($val) check is far easier (and quicker) in XS
> than an eval.

Makes sense. Though the lazy part of me wonders if it wouldn't be a
good idea to just ensure there's a freezer method in UNIVERSAL:

sub UNIVERSAL::freezer {
croak "WARNING(Freezer method call reached UNIVERSAL)";
shift;
}

which can be set up in the .pm module that eventually calls the 'real'
thing. Then, in the presence of a $Freezer variable, you just call the
method no matter what.