Skip to content

Dumper: object hash - #277

Merged
milo merged 1 commit into
nette:masterfrom
tsusanka:object-dump-md5
Nov 30, 2015
Merged

Dumper: object hash#277
milo merged 1 commit into
nette:masterfrom
tsusanka:object-dump-md5

Conversation

@tsusanka

Copy link
Copy Markdown
Contributor

If Assert:same() is used to compare two similiar-looking objects and the assert fails, the Tester generates two outputs which shows no difference at all.

This PR prints object's hash to distinguish the objects.

@Mikulas

Mikulas commented Nov 27, 2015

Copy link
Copy Markdown
Contributor

👍 good idea, I've definitely have seen identical output before and got stuck on it for a while.

Why do you hash the object hash with md5 though?

@dg

dg commented Nov 27, 2015

Copy link
Copy Markdown
Member

What about non-stdClass objects?

@tsusanka

Copy link
Copy Markdown
Contributor Author

@Mikulas I got inspired by Tester's Dumper.php. The sql_object_hash() returns "similiar" content to different objects, so it can't be easily stripped. See function's comment.

@dg this approach should work with non-stdClass objects as well, am I mistaken?

$ php -a
> class A {}
> $a = new A();
> echo spl_object_hash($a);
000000003ae624ea000000003019d5db

@Majkl578

Copy link
Copy Markdown
Contributor

this approach should work with non-stdClass objects as well

It's in $class === 'stdClass'branch.

@JanTvrdik

Copy link
Copy Markdown
Contributor

@tsusanka awesome! Maybe return "$class::__set_state(/* #$objHash */ array($out))" for other classes as well.

@tsusanka

Copy link
Copy Markdown
Contributor Author

Oh yeah, got it! Fixed.

@milo

milo commented Nov 30, 2015

Copy link
Copy Markdown
Member

@tsusanka 👍 Could you please:

  • separate it e.g. into private static function hash() (the # included in return value)
  • squash it into single commit with message like Dumper: print object hash into output dumps

@tsusanka

Copy link
Copy Markdown
Contributor Author

Thx for the instructions. Done.

milo added a commit that referenced this pull request Nov 30, 2015
@milo
milo merged commit 95012e7 into nette:master Nov 30, 2015
@milo

milo commented Nov 30, 2015

Copy link
Copy Markdown
Member

@tsusanka Thank you!

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.

6 participants