Project

General

Profile

Actions

task #7201

closed

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

Added by Andreas Müller about 6 years ago. Updated about 6 years ago.

Status:
Closed
Priority:
New
Category:
cdmlib
Target version:
Start date:
Due date:
% Done:

70%

Estimated time:
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 accountClosedAndreas Müller

Actions
Related to EDIT - feature request #7198: Implement equals() forCategoricalDataRejectedPatrick Plitzner

Actions
Related to EDIT - feature request #7199: Implement equals() for QuantitativeDataRejectedPatrick Plitzner

Actions
Related to EDIT - task #7202: [DISCUSS] Can we remove equals override from some classes directly inheriting from CdmBase NewAndreas Kohlbecker

Actions
Actions #1

Updated by Andreas Müller about 6 years ago

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

Updated by Andreas Müller about 6 years ago

Actions #3

Updated by Andreas Müller about 6 years ago

Actions #4

Updated by Andreas Müller about 6 years ago

AK:

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

Actions #5

Updated by Andreas Kohlbecker about 6 years 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?

Actions #6

Updated by Andreas Müller about 6 years 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).

Actions #7

Updated by Andreas Müller about 6 years ago

  • Target version changed from Unassigned CDM tickets to Release 4.13
Actions #8

Updated by Andreas Müller about 6 years 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).

Actions #9

Updated by Andreas Müller about 6 years ago

  • % Done changed from 0 to 70
Actions #10

Updated by Andreas Müller about 6 years ago

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

Updated by Katja Luther about 6 years 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

Actions

Also available in: Atom PDF