[ovirt-devel] Implementing equals & hashCode methods

[Yevgeny Zaspitsky]

See my comments in your mail body.

----- Original Message -----
> From: "Gilad Chaplik" <gchaplik at redhat.com>
> To: "Yevgeny Zaspitsky" <yzaspits at redhat.com>
> Cc: "Yair Zaslavsky" <yzaslavs at redhat.com>, devel at 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 at redhat.com>
> > To: "Yair Zaslavsky" <yzaslavs at redhat.com>
> > Cc: devel at 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 at redhat.com>
> > > To: "Yevgeny Zaspitsky" <yzaspits at redhat.com>
> > > Cc: devel at 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 at redhat.com>
> > > > To: devel at 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.

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
> > > > Devel at ovirt.org
> > > > http://lists.ovirt.org/mailman/listinfo/devel
> > > 
> > _______________________________________________
> > Devel mailing list
> > Devel at ovirt.org
> > http://lists.ovirt.org/mailman/listinfo/devel
> > 
>