Skip to content

review fix: CtExecutableReference have all the same hashcode #1717

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

Merged
merged 5 commits into from
Nov 13, 2017

Conversation

surli
Copy link
Collaborator

@surli surli commented Nov 13, 2017

It seems that Spoon make no distinction between reference hashcode: they all have a value to 1.

@surli
Copy link
Collaborator Author

surli commented Nov 13, 2017

@monperrus is it a wanted behaviour? It seems particularly weird to me, especially because it prevents us in using properly collection of references.

@monperrus
Copy link
Collaborator

@monperrus is it a wanted behaviour?

Not really

because it prevents us in using properly collection of references.

I'd say it still works correctly, but inefficiently (many calls to equals)

@surli
Copy link
Collaborator Author

surli commented Nov 13, 2017

I'd say it still works correctly

Why "correctly"? here the test shows that two completely distinct executable references have the same hashcode...

@surli
Copy link
Collaborator Author

surli commented Nov 13, 2017

I propose a fix here, but I'm not sure it's enough: several elements in Spoon may have the same hashCode issue.

@monperrus
Copy link
Collaborator

Why "correctly"?

A hashCode is an approximation in essence. It's normal that you sometimes have collisions and the collection libraries know how to handle this. You have bad performance if you have too many collisions, but you have correct execution.

@monperrus
Copy link
Collaborator

This fix looks good to me.

@surli surli changed the title WiP fix: CtExecutableReference have all the same hashcode review fix: CtExecutableReference have all the same hashcode Nov 13, 2017
@surli
Copy link
Collaborator Author

surli commented Nov 13, 2017

@monperrus it's ready for merging for me.

@surli surli force-pushed the fix-reference-hashcode-are-identical branch from 9f96507 to 59b26c7 Compare November 13, 2017 13:46
@surli
Copy link
Collaborator Author

surli commented Nov 13, 2017

The CI is ok: we have an issue with gakoci because Java 8 release 151 seems not available through maven docker image easily. If you're ok I propose that you merge this one as I need it for #1707

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants