Assert::isEqual() can compare objects that are recursive

[fprochazka]

Without this pullrequest

use Nette\Forms\Controls\TextInput;

Assert::equal(new TextInput(), new TextInput());
// Exception('Nesting level too deep or recursive dependency.');

and with

Assert::equal(new TextInput(), new TextInput());
// success

[fprochazka]

Please :) https://travis-ci.org/Kdyby/DoctrineForms/jobs/18951654

[milo]

@fprochazka Sorry, but I'm not sure if it can be implemented this way. It is a question about "what equality means" in this case.

Following code pass even reference points to other structure part.

$r1 = new stdClass;
$r1->a = new stdClass;
$r1->a->a = $r1;

$r2 = new stdClass;
$r2->a = new stdClass;
$r2->a->a = $r2->a;

Maybe references should be replaced by structure path before comparing. And it would be great for arrays too :)

[fprochazka]

@milo technically, you're checking equality of given structures, and because they don't have any other properties, they are equal.

$r1 = new stdClass;
$r1->b = 'b';
$r1->a = new stdClass;
$r1->a->a = $r1;
$r1->a->c = 'c';

$r2 = new stdClass;
$r2->b = 'b';
$r2->a = new stdClass;
$r2->a->a = $r2->a;
$r2->a->c = 'c';

Assert::equal($r1, $r2);

If you add values to the structure it's not equal anymore.

[fprochazka]

@dg what do you think?

[dg]

I think that "equal" is used in cases, when we want to ignore objects's identity. So in @milo structure should be ignored identity of $rx->a (so they are equal), but in @fprochazka they are not equal, because $r1->a->a->b = 'b' and $r2->a->a->b = undefined

[fprochazka]

@dg what is your suggestion? There is no reason we should be throwing recursion exception if it can be checked this easily.

In fact, those two structures are perfectly equal untill you add other values to them (just like you did). @milo's example is equal, mine is not, yours is not.

There is infinite recursion of objects that have only one property a and that property equals to instance of stdClass that has only one property a and that property equals to instance of stdClass, ...

They are equal untill you add other properties to them. I haven't had a high school math so I might be wrong, but in what universe those two structures are not equal?

---

These are equal

$r1 = new stdClass;
$r1->a = new stdClass;
$r1->a->a = $r1;

$r2 = new stdClass;
$r2->a = new stdClass;
$r2->a->a = $r2->a;

These are not

$r1 = new stdClass;
$r1->b = 'b';
$r1->a = new stdClass;
$r1->a->a = $r1;

$r2 = new stdClass;
$r2->b = 'b';
$r2->a = new stdClass;
$r2->a->a = $r2->a;

---

The only think I did was that if the algorithm finds an object that is already beeing checked (in higher call) it assumes that the object is equal to the other side, because if not, it will fail on the higher check.

It simply prevents double checking of the object properties (and as a side effect fixes object-recursion problem)

[milo]

I think that base problem is definition of back-reference equality. Imagine it as paths:

$r1 = new stdClass;
$r1->a = new stdClass;
$r1->a->a = $r1; # = /

$r2 = new stdClass;
$r2->a = new stdClass;
$r2->a->a = $r2->a; # = /a/

I prepared another point of view solution. I'll send it during evening.

[castamir]

Problem is "where to stop" (its halting problem). It basically means that you may stop but you might not.

In your case, it will stop, but in another (basically the same) it might not (you will probably run out of memory before you find any relation between nested objects):

$r1 = new stdClass;
$r1->a = new stdClass;
$r1->a->a = new stdClass;
$r1->a->a->a = new stdClass;
$r1->a->a->a->a = new stdClass;
$r1->a->a->a->a->a = new stdClass;
...
$r1->a->a->a->a->a->a...->a = $r1;

$r2 = new stdClass;
$r2->a = new stdClass;
$r2->a->a = $r2->a;

[fprochazka]

@castamir in that case the Assert::equals() method will say there is recursion that cannot be solved and throws exception.

I don't see myself solving such absurd situations, I just wanna be able to compare equality of two normal objects (like TextInput).

The beauty of this is that we don't have to solve all possible structures there ever will be, we have to only solve few standard cases and in the rest we can throw new NotImplementedException("Wait for it... or implement it yourself").

[milo]

@fprochazka @dg What about this solution?

[fprochazka]

@milo my brain hurts from all those testcases but I guess it makes sense :) And it works in my use-case (I've tested it), so as far as my vote goes, I'm in :)

[dg]

I would rather prefer @milo's approach, #99.

[fprochazka]

@dg I like the hashes more, I was thinking about them before I started writing the code, but I had a hunch you might wanna avoid them (I don't know how fast they are).

[dg]

Speed absolutely doesn't matter here. But I changed it to SplObjectStorage for better readability.

Btw simpler your solution, without hashes:

            if ($expected === $actual || in_array([$expected, $actual], $checked, TRUE)) {
                return TRUE;
            }
            $checked[] = [$expected, $actual];

[fprochazka]

Closing in favor of #99