Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dumper: object hash #277

Merged
merged 1 commit into from Nov 30, 2015
Merged

Dumper: object hash #277

merged 1 commit into from Nov 30, 2015

Conversation

@tsusanka
Copy link
Contributor

tsusanka commented Nov 27, 2015

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

This comment has been minimized.

Copy link
Contributor

Mikulas commented Nov 27, 2015

👍 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

This comment has been minimized.

Copy link
Member

dg commented Nov 27, 2015

What about non-stdClass objects?

@tsusanka

This comment has been minimized.

Copy link
Contributor Author

tsusanka commented Nov 27, 2015

@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

This comment has been minimized.

Copy link
Contributor

Majkl578 commented Nov 27, 2015

this approach should work with non-stdClass objects as well

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

@JanTvrdik

This comment has been minimized.

Copy link
Contributor

JanTvrdik commented Nov 27, 2015

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

@tsusanka

This comment has been minimized.

Copy link
Contributor Author

tsusanka commented Nov 28, 2015

Oh yeah, got it! Fixed.

@milo

This comment has been minimized.

Copy link
Member

milo commented Nov 30, 2015

@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 tsusanka force-pushed the tsusanka:object-dump-md5 branch from b6eeca8 to 27428f5 Nov 30, 2015
@tsusanka

This comment has been minimized.

Copy link
Contributor Author

tsusanka commented Nov 30, 2015

Thx for the instructions. Done.

milo added a commit that referenced this pull request Nov 30, 2015
Dumper: object hash
@milo milo merged commit 95012e7 into nette:master Nov 30, 2015
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@milo

This comment has been minimized.

Copy link
Member

milo commented Nov 30, 2015

@tsusanka Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.