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

Rename hashcode method to hashCode to override Object.hashCode() #320

Merged
merged 1 commit into from
Feb 13, 2017
Merged

Rename hashcode method to hashCode to override Object.hashCode() #320

merged 1 commit into from
Feb 13, 2017

Conversation

szarnyasg
Copy link
Contributor

The hashCode() method in the InternalRecord class is spelt with a lowercase c, hence it is not executed when running hashCode(). Instead, the Object.hashCode() method is executed, which does not work well for InternalRecord instances and gives different hash values for tuples with the same key-value pairs.

This pull request fixes that by renaming the method appropriately.

Additionally, the Record interface tries to enforce the implementation of equals() and hashCode(). However, it seems it is not possible to enforce the implementation of the these methods by using an interface - if they are not overridden in the class, the implementation of the Object method will be used.

As a minimal example, this interface and its implementation compile without errors.

public interface MyInterface {

    @Override
    int hashCode();

}

public class MyImplementation implements MyInterface {

}

One could enforce their implementation is to use an abstract class. The following code does not compile.

public abstract class MyAbstractClass {

    @Override
    public abstract int hashCode();

}


public class MyConcreteClass extends MyAbstractClass {

}

However, this solution cannot be applied to Record as the InternalRecord class already implements another class (InternalMapAccessorWithDefaultValue).

In this pull request, I also deleted the @Override definitions for equals and hashCode in the Record interface as they basically have no effect.

@lutovich
Copy link
Contributor

Hi @szarnyasg.

Thanks a lot for the PR! Changes look good, this is clearly an oversight from our side. Could you please also add @Override annotation to #equals() and #hashCode() methods in InternalRecord?

To be able to merge your changes we will need a signed CLA from you. See http://neo4j.com/developer/cla/ for further information. Additional information on contributing code is found at http://neo4j.com/developer/contributing-code/.

@szarnyasg
Copy link
Contributor Author

@lutovich I emailed a CLA to cla@neotechnology.com last year with the following timestamp: Sat, Sep 24, 2016 at 4:18 PM, but did not receive a response. Do I have to print the PDF, undersign on paper and scan it? This was not entirely clear for me from the CLA guide.

I'll add the changes in a minute.

@szarnyasg
Copy link
Contributor Author

Update: I added the override annotations.

@lutovich
Copy link
Contributor

@szarnyasg I'm sorry but I can't see your CLA in the inbox. Could you please resend it? You need to only attach that PDF to the letter, no need to print and scan it.

@lutovich
Copy link
Contributor

@szarnyasg found your CLA. Tried to search for your name but it is spelled differently in the CLA inbox and on GitHub :)
Will start a new PR build and merge when it turns green. Thanks!

@szarnyasg
Copy link
Contributor Author

Thanks - the wonders of accented characters :-). I'll pay attention to this in the future.

@lutovich lutovich merged commit 77ed0e5 into neo4j:1.1 Feb 13, 2017
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.

2 participants