Project

General

Profile

task #7201

[DISCUSS] Should we remove created comparison from CdmBase.equals?

Added by Andreas Müller 11 months ago. Updated 10 months ago.

Status:
Closed
Priority:
New
Category:
cdmlib
Target version:
Start date:
01/18/2018
Due date:
% Done:

70%

Severity:
normal

Description

For discussion on equals see also #7155 and related tickets.

Does anybody remember a reason why created is compared in CdmBase.equals?


Related issues

Related to Edit - bug #7155: VersionableEntity.equals() should take updated timestamp into account Closed 01/08/2018
Related to Edit - feature request #7198: Implement equals() forCategoricalData Rejected 01/17/2018
Related to Edit - feature request #7199: Implement equals() for QuantitativeData Rejected 01/17/2018
Related to Edit - task #7202: [DISCUSS] Can we remove equals override from some classes directly inheriting from CdmBase New 01/18/2018

Associated revisions

Revision 579a4a81 (diff)
Added by Andreas Müller 11 months ago

ref #7201 remove created from equals and remove equals override in TermBase

  • the new CdmBase.equals implementation should now be equivalent to old TermBase.equals

Revision 5069790f (diff)
Added by Andreas Müller 11 months ago

ref #7201, ref #7155 remove equals override also from OrderedTermBase

Revision bef37262 (diff)
Added by Andreas Müller 11 months ago

ref #7201, ref #7155 javadoc

Revision 04af63c0 (diff)
Added by Andreas Müller 11 months ago

ref #7201, ref #7155 revert setting to final in CdmBase (was not on purpose)

History

#1 Updated by Andreas Müller 11 months ago

  • Related to bug #7155: VersionableEntity.equals() should take updated timestamp into account added

#2 Updated by Andreas Müller 11 months ago

#3 Updated by Andreas Müller 11 months ago

#4 Updated by Andreas Müller 11 months ago

AK:

Ich erinnere mich an keinen Grund weshalb das Create verglichen wird. Falls mir noch einer einfällt sage ich Bescheid.

#5 Updated by Andreas Kohlbecker 11 months ago

Andreas Müller wrote:

AK:

Ich erinnere mich an keinen Grund weshalb das Create verglichen wird. Falls mir noch einer einfällt sage ich Bescheid.
Vielleicht um "re-importierte" entities unterscheiden zu können? Diese hätten die selbe UUID aber eine andere id. Würden in der equals methode die ids verglichen wäre dies nicht nötig.

Die Implementierung clone Methode ist in dieser Hinsicht ganz interessant:

 result.setId(0);
        result.setUuid(UUID.randomUUID());
        result.setCreated(new DateTime());
        result.setCreatedBy(null);

demnach wären in der derzeitigen Implementierung Klone nicht equal zum Original, ich denke das macht Sinn.

Ach übrigens, da wir schon beim Thema sind, in https://developer.jboss.org/wiki/EqualsAndHashCode wird darauf hingewiesen, dass equals() und hashCode() immer äquivalent implementiert werden sollten. hashCode() wird in CmdBase aber nicht überschrieben. Ist das bewusst so o der wurde das vergessen?

#6 Updated by Andreas Müller 11 months ago

Andreas Kohlbecker wrote:

Ach übrigens, da wir schon beim Thema sind, in https://developer.jboss.org/wiki/EqualsAndHashCode wird darauf hingewiesen, dass equals() und hashCode() immer äquivalent implementiert werden sollten. hashCode() wird in CmdBase aber nicht überschrieben. Ist das bewusst so o der wurde das vergessen?

Doch, es wird überschrieben. Steht direkt unter equals. Dort wird überigens auch nur die uuid für die Berechnung des HashCodes verwendet. Spricht auch dafür, dass wir das created rausnehmen sollten (oder mit in die HashCode Berechnung aufnehmen sollten).

#7 Updated by Andreas Müller 11 months ago

  • Target version changed from Unassigned CDM tickets to Release 4.13

#8 Updated by Andreas Müller 11 months ago

  • Status changed from New to Resolved
  • Assignee changed from Andreas Müller to Katja Luther

As TermBase and OrderedTermBase also had there own implementation but without created comparison I did now remove the created comparison from CdmBase.equals().

So everybode should carefully check if everything still works as expected, also with DefinedTerms and OrderedTerms as I did remove the equals implementation from their to be able to set equals to final in VersionableEntity (and maybe later also in CdmBase).

#9 Updated by Andreas Müller 11 months ago

  • % Done changed from 0 to 70

#10 Updated by Andreas Müller 11 months ago

  • Related to task #7202: [DISCUSS] Can we remove equals override from some classes directly inheriting from CdmBase added

#11 Updated by Katja Luther 10 months ago

  • Status changed from Resolved to Closed
  • Assignee changed from Katja Luther to Andreas Müller

seems logically correct and as long as I see everything works

Also available in: Atom PDF

Add picture from clipboard (Maximum size: 40 MB)