Skip to content

Assert::isEqual: can compare recursive objects [Closes #93] - #99

Merged
dg merged 2 commits into
nette:masterfrom
dg:pull-recursive
Feb 19, 2014
Merged

dg merged 2 commits into
nette:masterfrom
dg:pull-recursive

Conversation

@dg

@dg dg commented Feb 18, 2014

Copy link
Copy Markdown
Member

No description provided.

Comment thread Tester/Framework/Assert.php Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are you sure about this one? I think there should be &&

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why && ?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know, I just woke up and it looks weird :)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't there be the path problem that milo tried to solve?

@fprochazka

Copy link
Copy Markdown
Contributor

@milo any objections?

@milo

milo commented Feb 18, 2014

Copy link
Copy Markdown
Member

Nice and elegant! I had to sit and draw it on paper :) Comments in code.

Comment thread Tester/Framework/Assert.php Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe shorter:

            if (isset($checked[$expected])) {
                return $checked[$expected] === $actual;
            } elseif (isset($checked[$actual])) {
                return FALSE;
            } elseif ($expected === $actual) {
                return TRUE;
            }

Btw. second elseif seems logically, but test passes without it too. It just returns FALSE on next level.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, in this case the second if is unnecessary, because $expected !== $actual. Repushed.

Now it is really very simple implementation ;-)

@milo

milo commented Feb 19, 2014

Copy link
Copy Markdown
Member

Great. I would merge it soon if no one complains.

@fprochazka

Copy link
Copy Markdown
Contributor

👍

dg added a commit that referenced this pull request Feb 19, 2014
Assert::isEqual: can compare recursive objects [Closes #93]
@dg
dg merged commit d789015 into nette:master Feb 19, 2014
@fprochazka

Copy link
Copy Markdown
Contributor

👏

@dg
dg deleted the pull-recursive branch March 23, 2014 22:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants