3:18 a.m.
----- Original Message -----
From: "Yevgeny Zaspitsky" <yzaspits(a)redhat.com>
To: "Gilad Chaplik" <gchaplik(a)redhat.com>
Cc: devel(a)ovirt.org
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" <gchaplik(a)redhat.com>
> To: "Yevgeny Zaspitsky" <yzaspits(a)redhat.com>
> Cc: "Yair Zaslavsky" <yzaslavs(a)redhat.com>, 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>
> > To: "Yair Zaslavsky" <yzaslavs(a)redhat.com>
> > 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>
> > > To: "Yevgeny Zaspitsky" <yzaspits(a)redhat.com>
> > > 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>
> > > > 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.
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
> > > > 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
> >
>
_______________________________________________
Devel mailing list
Devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel