Re: [ovirt-devel] Implementing equals & hashCode methods

From: "Gilad Chaplik" <gchaplik(a)redhat.com&gt; To: "Yevgeny Zaspitsky" <yzaspits(a)redhat.com&gt; Cc: "Yair Zaslavsky" <yzaslavs(a)redhat.com&gt;, devel(a)ovirt.org Sent: Sunday, May 25, 2014 8:41:36 PM Subject: Re: [ovirt-devel] Implementing equals & hashCode methods Thanks, Gilad. ----- Original Message ----- > From: "Yevgeny Zaspitsky" <yzaspits(a)redhat.com&gt; > To: "Yair Zaslavsky" <yzaslavs(a)redhat.com&gt; > Cc: devel(a)ovirt.org > Sent: Sunday, May 25, 2014 4:52:30 PM > Subject: Re: [ovirt-devel] Implementing equals & hashCode methods > > See my comments in your mail body. > > ----- Original Message ----- > > From: "Yair Zaslavsky" <yzaslavs(a)redhat.com&gt; > > To: "Yevgeny Zaspitsky" <yzaspits(a)redhat.com&gt; > > Cc: devel(a)ovirt.org > > Sent: Sunday, May 25, 2014 4:09:11 PM > > Subject: Re: [ovirt-devel] Implementing equals & hashCode methods > > > > > > > > ----- Original Message ----- > > > From: "Yevgeny Zaspitsky" <yzaspits(a)redhat.com&gt; > > > To: devel(a)ovirt.org > > > Sent: Sunday, May 25, 2014 3:33:31 PM > > > Subject: [ovirt-devel] Implementing equals & hashCode methods > > > > > > Hi All, > > > > > > Recently I reviewed a patch that adds a new business entity to the > > > engine. > > > The entity class has the following members: > > > > > > * id > > > * data center id > > > * name > > > * type > > > * some other properties that do not belong to the entity key > > > > > > The equals & hashCode methods were implemented in a way that include > > > all > > > members. > > > I asked the patch author to change that, so it'll include only business > > > key > > > (data center id, name and type), which define the entity uniqueness. > > > Also I found that many other business entities are implemented in a > > > similar > > > way (include all class members in equals & hashCode). > > > > > > I'm a new to oVirt, so I'd like to ask your opinion on the issue. > > > > > > > > > 1. Do you agree with my approach on equals & hashCode. > > > 2. If you agree with my approach in general, should we implement it > > > in > > > the new introduced code or should we adhere to the old convention > > > even > > > we do not agree with it? > > > 3. Should we re-factor the old code (it might be dangerous as we do > > > have > > > enough unit test coverage)? I agree it is a very important issue, but since oVirt 3.5 is a right at the door, I suggest to postpone this discussion to quieter times :-) You are more than welcome to submit a fix - it'd most appreciated.

Thanks, Gilad. > > > > > > Thanks in advance, > > > Yevgeny > > > > I assume your idea came (at least from ) the way identify is defined at > > Hibernate. > > The advantages are obvious - > > a. shorter code > > b. let's time to compute > > > > However, I fear this is may lead to some bugs if identities are not > > defined > > well. > > We should strive to define our classes well. Actually that point refers to > the design stage rather than to the implementation. > > On other hand, if we put the objects (that include all members in their > equals) in a HashSet in order to keep unique instances, we might have a bug > there. > > > > > Regarding 3 - might be dangerous as we don't have enough unit test > > coverage, > > probably. > > Actually, IMHO the situation with DAO tests which uses equals is quite > > good. > > Those classes are used all over engine code and even in the UI code, so > changing the implementation in the existing code is very risky. Having DAO > covered isn't enough IMHO. > > > For hashCode - I assume you have a point > > As you probably know hashCode implementation could not be (conceptually) > different from the equals one. > > > > > > > > > > > > > _______________________________________________ > > > Devel mailing list > > > Devel(a)ovirt.org > > > http://lists.ovirt.org/mailman/listinfo/devel > > > _______________________________________________ > Devel mailing list > Devel(a)ovirt.org > http://lists.ovirt.org/mailman/listinfo/devel >