Modify

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#19281 closed enhancement (fixed)

[PATCH] Use Objects.hash where it is not used

Reported by: hiddewie Owned by: team
Priority: trivial Milestone: 20.05
Component: Core Version:
Keywords: refactoring Cc:

Description

In many of the JOSM classes, the Objects.hash(...) function has been used to simplify the generation of an int hashCode() method.

This patch uses this in some leftover methods that still had a generated implementation.

Attachments (1)

Objects_hash_simplifications1.patch (23.6 KB ) - added by hiddewie 4 years ago.

Download all attachments as: .zip

Change History (12)

by hiddewie, 4 years ago

comment:1 by simon04, 4 years ago

Keywords: refactoring added
Milestone: 20.05

Parent ticket: #19174

We don't have to keep the >>> 32, this originates from java.lang.Double#hashCode(double) and will be called when using Objects.hash. I'll update your patch and commit it.

Thank you!

comment:2 by simon04, 4 years ago

Resolution: fixed
Status: newclosed

In 16488/josm:

fix #19281, see #19174 - Use Objects.hash where it is not used (patch by hiddewie, modified)

comment:4 by GerdP, 4 years ago

Simple typo?
return Objects.hash(lhs, lhs);
should be
return Objects.hash(lhs, rhs);

comment:5 by GerdP, 4 years ago

In 16491/josm:

see #19281: fix typo

comment:6 by GerdP, 4 years ago

In 16493/josm:

see #19281: fix another typo reported by EqualsVerifier (super.hashCode was missing)

comment:7 by hiddewie, 4 years ago

Thanks for those fixes. Is there a way to display static analysis and test results before patches get merged? This would prevent errors in the 'master' branch.

comment:8 by GerdP, 4 years ago

I only know the ant tasks checkstyle, checkstyle-changed, spotbugs, and pmd and of course clean dist to begin with.

comment:9 by simon04, 4 years ago

@GerdP, thank you!

Those testEqualsContract are standard unit tests (test/unit/org/openstreetmap/josm/data/osm/search/SearchCompilerTest.java). Those can be run from the IDE, or from the commandline using the following command:

ant test '-Ddefault-junit-includes=**/SearchCompilerTest.class'

comment:10 by hiddewie, 4 years ago

Yes I did not run all tests locally and should have of course.

My question was more about a visual feedback within this ticket about test results from CI, before they are merged to the master branch. I see there is work going on to migrate to a GitLab instance (#16871) which would give such functionality out of the box.

I am willing to help setting up some pipelines up in GitLab if there are concrete tasks.

comment:11 by simon04, 4 years ago

The GitLab server is not ready to be used, unfortunately.

What we've got in place at the moment is Travis CI setup for the GitHub mirror – https://travis-ci.org/github/openstreetmap/josm. As you can tell from the number of failing builds, it hasn't gotten any love in the past months. Opening a PR for https://github.com/openstreetmap/josm triggers this CI, however, since it is just a mirror, we cannot merge the PRs via GitHub but have to apply them manually via SVN.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain team.
as The resolution will be set.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.