Thanks, Gilad.
----- Original Message ----- > From: "Yevgeny Zaspitsky" <[email protected]> > To: "Yair Zaslavsky" <[email protected]> > Cc: [email protected] > 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" <[email protected]> > > To: "Yevgeny Zaspitsky" <[email protected]> > > Cc: [email protected] > > Sent: Sunday, May 25, 2014 4:09:11 PM > > Subject: Re: [ovirt-devel] Implementing equals & hashCode methods > > > > > > > > ----- Original Message ----- > > > From: "Yevgeny Zaspitsky" <[email protected]> > > > To: [email protected] > > > 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 > > > [email protected] > > > http://lists.ovirt.org/mailman/listinfo/devel > > > _______________________________________________ > Devel mailing list > [email protected] > http://lists.ovirt.org/mailman/listinfo/devel > _______________________________________________ Devel mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/devel
