----- Original Message ----- > From: "Yevgeny Zaspitsky" <[email protected]> > To: "Gilad Chaplik" <[email protected]> > Cc: [email protected] > Sent: Monday, May 26, 2014 10:25:08 AM > Subject: Re: [ovirt-devel] Implementing equals & hashCode methods > > See my comments in your mail body. > > ----- Original Message ----- > > From: "Gilad Chaplik" <[email protected]> > > To: "Yevgeny Zaspitsky" <[email protected]> > > Cc: "Yair Zaslavsky" <[email protected]>, [email protected] > > Sent: Sunday, May 25, 2014 8:41:36 PM > > Subject: Re: [ovirt-devel] Implementing equals & hashCode methods > > > > > > > > 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.
Yes, when we have a single UUID/Long key (see my comment in 3) > > > > > 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? Change it when applicable > > > > > 3. Should we re-factor the old code (it might be dangerous as we > > > > > do > > > > > have > > > > > enough unit test coverage)? Yes, we just have to avoid that when the key is not a single column UUID/long and it is actually a collection of columns (i.e. columnA,columnB for example) since this key may be subjected to a change (for example adding another column to the key) > > > > 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. > > Gilad, thank you for opinion even though it's not very relevant to the > thread. > > > > > 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 > _______________________________________________ Devel mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/devel
