Ah, I was too focused on compareTo() which doesn't handle nulls...  :-)

On Tue, Jun 9, 2020 at 8:15 AM Andrus Adamchik <and...@objectstyle.org>
wrote:

> It should not. "instanceof" takes care of weeding out nulls.
>
>
> > On Jun 9, 2020, at 3:11 PM, Michael Gentry <blackn...@gmail.com> wrote:
> >
> > BTW, I believe your current implementation will NPE on a null o2 here:
> >
> > return ((BigDecimal) o1).compareTo((BigDecimal) o2) == 0;
> >
> >
> >
> > On Fri, Jun 5, 2020 at 11:34 AM Michael Gentry <blackn...@gmail.com>
> wrote:
> >
> >> Will adding this work?  (Worked in a simple test I wrote.)
> >>
> >>    public static boolean nullSafeEquals(BigDecimal o1, BigDecimal o2)
> >>    {
> >>        if (o1 == null || o2 == null)
> >>            return o1 == o2;
> >>
> >>        return o1.compareTo(o2) == 0;
> >>    }
> >>
> >> Then delete the BigDecimal stuff from the other nullSafeEquals.  (Revert
> >> the changes.)
> >>
> >>
> >>
> >> On Fri, Jun 5, 2020 at 8:47 AM Andrus Adamchik <and...@objectstyle.org>
> >> wrote:
> >>
> >>> Hi folks,
> >>>
> >>> Just ran into an obscure and minor but at the same time fundamental
> bug -
> >>> Cayenne generated unneeded updates when the only change is BidDecimal
> value
> >>> scale [1]. Essentially for BigDecimals we shouldn't be calling
> >>> "a.equals(b)" but use "a.compareTo(b) == 0". I have a fix [2], but
> using
> >>> "instanceof" inside a common performance-sensitive code feels dirty.
> I'd
> >>> much rather a comparison strategy to be a part of the
> PropertyDescriptor
> >>> object or something. So not applying the PR just yet.
> >>>
> >>> Wanted to mention it here. Perhaps there are other data types that
> can't
> >>> use "equals" for some reason. And perhaps there are other solutions to
> this
> >>> problem too...
> >>>
> >>> Andrus
> >>>
> >>> [1] https://issues.apache.org/jira/browse/CAY-2660
> >>> [2] https://github.com/apache/cayenne/pull/426
> >>
> >>
>
>

Reply via email to