Re: [code-review] Please review String::FlexMatch

Tony Bowden <[email protected]> Sat, 6 Sep 2003 12:44:13 +0100
Newsgroups gmane.comp.lang.perl.code-review-ladder
Message-ID <[email protected]>
On Sat, Sep 06, 2003 at 10:52:55AM +0100, Fergal Daly wrote:
> Below are some more arguments for why is_deeply shouldn't do it.
> But the object isn't asking for string-wise equality, it's asking for deep 
> equality and the only thing that's deeply equal to "string" is "string", not 
> an object that is pretending to be "string". Deep is the key word here.

I still maintain that your concept of 'deep' here is wrong.


> The docs don't say anything about string equality, they say that is_deeply 
> will walk the structures and that should include the structures of an
> overloaded object.

Why? 

Consider the following:

	package MyString;

	use overload
		'""' => sub { shift->{value} },
		fallback => 1;

	sub new {
		my ($class, $val) = @_;
		bless { value => $val }, $class;
	}

	package main;

	use Test::More tests => 2;

	my $fred = MyString->new("fred");

	is $fred, "fred";
	is_deeply [$fred], ["fred"];

Do you really think that test 1 should pass and test 2 should fail?

If so, what about the case where overloaded constants are in operation
and my $fred = "fred" creates an object ...

> What if you had overloaded eq on $ref and were accidentally always returning 
> true? is_deeply($ref, "any string") would be a pass.

Yes. But that would be the *correct* behaviour.

> I thought of another example of why it's wrong. Take $o1, $o2 and "string". 
> Say $o1 and $o2 both stringify to "string" but they are completely different 
> objects so. $o1 is deeply equal to "string", "string" is deeply equal to $o2 
> but $o1 and $o2 are not deeply equal to each other. This is not good.

Yes, it is. 

If you want to be twiddling with the innards of objects to see if
they're the same then you should be using something different.

is_deeply() should just be using is(). If is() says they're the same,
then is_deeply should say they're the same if they occur at the same
place in a list/hash/whatever.

> Like I said, I've asked Michael Schwern for his take.

Like I said, he's given it:

	> is_deeply [ State->columns('Primary') ] => [qw/name/],
	>   'State Primary:' . join ", ", State->columns('Primary');

	This should work.  The test should not ignore stringification otherwise
	you can't do black box testing using is_deeply().

     https://rt.cpan.org/Ticket/Display.html?id=3690


Tony