[ovirt-devel] Implementing equals & hashCode methods

[Gilad Chaplik]

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.

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
>