Change BigDecimal constructor in GenericEntity

[Jacopo Cappellato]

What do you think about the attached patch?
I really think it should be committed to the trunk, especially after
reading the following notes:

http://java.sun.com/j2se/1.5.0/docs/api/java/math/BigDecimal.html#BigDecimal(double)

However, I'd like to get feedback from you before committing it since it
is a low level change.

Jacopo

PS: thanks to Martin Anderson from the patch and research around it.

Index: framework/entity/src/org/ofbiz/entity/GenericEntity.java
===================================================================
--- framework/entity/src/org/ofbiz/entity/GenericEntity.java (revisione 576353)
+++ framework/entity/src/org/ofbiz/entity/GenericEntity.java (copia locale)
@@ -588,7 +588,7 @@
         // NOTE: for things to generally work properly BigDecimal should really be used as the java-type in the field type def XML files
         Object value = get(name);
         if (value instanceof Double) {
-            return new BigDecimal(((Double) value).doubleValue());
+            return new BigDecimal(((Double) value).toString());
         } else {
             return (BigDecimal) get(name);
         }

[Jacques Le Roux]

Jacopo,

In this article, it's about canonical representation ? Could you be more precise please ?

So this mean that BigDecimal will be used as java-type in entitymodel.xml files (and in most places will replace Double) ?

Does this mean that "Double/BigDecimal issues are all resolved" ? If yes, then +1 ... (I saw that 2 issues are still pending after
Scoot recent work)

Jacques

De : "Jacopo Cappellato" <[hidden email]>

> What do you think about the attached patch?
> I really think it should be committed to the trunk, especially after
> reading the following notes:
>
> http://java.sun.com/j2se/1.5.0/docs/api/java/math/BigDecimal.html#BigDecimal(double)
>
> However, I'd like to get feedback from you before committing it since it
> is a low level change.
>
> Jacopo
>
> PS: thanks to Martin Anderson from the patch and research around it.
>

--------------------------------------------------------------------------------


> Index: framework/entity/src/org/ofbiz/entity/GenericEntity.java
> ===================================================================
> --- framework/entity/src/org/ofbiz/entity/GenericEntity.java (revisione 576353)
> +++ framework/entity/src/org/ofbiz/entity/GenericEntity.java (copia locale)
> @@ -588,7 +588,7 @@
>          // NOTE: for things to generally work properly BigDecimal should really be used as the java-type in the field type def
XML files
>          Object value = get(name);
>          if (value instanceof Double) {
> -            return new BigDecimal(((Double) value).doubleValue());
> +            return new BigDecimal(((Double) value).toString());
>          } else {
>              return (BigDecimal) get(name);
>          }
>

[Scott Gray]

Hi Jacques

The two remaining deprecated methods were unrelated to the BigDecimal/double
work, however there is still a ton of places where doubles are being used
instead of BigDecimal.

Anyway, this looks to be more about how to create a BigDecimal from a Double
rather than anything to do with converting existing code.

Regards
Scott

On 18/09/2007, Jacques Le Roux <[hidden email]> wrote:

>
> Jacopo,
>
> In this article, it's about canonical representation ? Could you be more
> precise please ?
>
> So this mean that BigDecimal will be used as java-type in entitymodel.xmlfiles (and in most places will replace Double) ?
>
> Does this mean that "Double/BigDecimal issues are all resolved" ? If yes,
> then +1 ... (I saw that 2 issues are still pending after
> Scoot recent work)
>
> Jacques
>
> De : "Jacopo Cappellato" <[hidden email]>
> > What do you think about the attached patch?
> > I really think it should be committed to the trunk, especially after
> > reading the following notes:
> >
> >
> http://java.sun.com/j2se/1.5.0/docs/api/java/math/BigDecimal.html#BigDecimal(double)
> >
> > However, I'd like to get feedback from you before committing it since it
> > is a low level change.
> >
> > Jacopo
> >
> > PS: thanks to Martin Anderson from the patch and research around it.
> >
>
>
>
> --------------------------------------------------------------------------------
>
>
> > Index: framework/entity/src/org/ofbiz/entity/GenericEntity.java
> > ===================================================================
> > --- framework/entity/src/org/ofbiz/entity/GenericEntity.java (revisione
> 576353)
> > +++ framework/entity/src/org/ofbiz/entity/GenericEntity.java (copia
> locale)
> > @@ -588,7 +588,7 @@
> >          // NOTE: for things to generally work properly BigDecimal
> should really be used as the java-type in the field type def
> XML files
> >          Object value = get(name);
> >          if (value instanceof Double) {
> > -            return new BigDecimal(((Double) value).doubleValue());
> > +            return new BigDecimal(((Double) value).toString());
> >          } else {
> >              return (BigDecimal) get(name);
> >          }
> >
>
>

[Jacques Le Roux]

Thanks Scott,

De : "Scott Gray" <[hidden email]>
> Hi Jacques
>
> The two remaining deprecated methods were unrelated to the BigDecimal/double
> work, however there is still a ton of places where doubles are being used
> instead of BigDecimal.

Yes indeed, in the POS module for instance BigDecimal are not used at all. I opened a Jira issue for that.

> Anyway, this looks to be more about how to create a BigDecimal from a Double
> rather than anything to do with converting existing code.

I had not time to look at it seriously (hence my questions), I will have a closer look tomorrow morning.

Jacques

> Regards
> Scott
>
> On 18/09/2007, Jacques Le Roux <[hidden email]> wrote:
> >
> > Jacopo,
> >
> > In this article, it's about canonical representation ? Could you be more
> > precise please ?
> >
> > So this mean that BigDecimal will be used as java-type in entitymodel.xmlfiles (and in most places will replace Double) ?
> >
> > Does this mean that "Double/BigDecimal issues are all resolved" ? If yes,
> > then +1 ... (I saw that 2 issues are still pending after
> > Scoot recent work)
> >
> > Jacques
> >
> > De : "Jacopo Cappellato" <[hidden email]>
> > > What do you think about the attached patch?
> > > I really think it should be committed to the trunk, especially after
> > > reading the following notes:
> > >
> > >
> > http://java.sun.com/j2se/1.5.0/docs/api/java/math/BigDecimal.html#BigDecimal(double)
> > >
> > > However, I'd like to get feedback from you before committing it since it
> > > is a low level change.
> > >
> > > Jacopo
> > >
> > > PS: thanks to Martin Anderson from the patch and research around it.
> > >
> >
> >
> >
> > --------------------------------------------------------------------------------
> >
> >
> > > Index: framework/entity/src/org/ofbiz/entity/GenericEntity.java
> > > ===================================================================
> > > --- framework/entity/src/org/ofbiz/entity/GenericEntity.java (revisione
> > 576353)
> > > +++ framework/entity/src/org/ofbiz/entity/GenericEntity.java (copia
> > locale)
> > > @@ -588,7 +588,7 @@
> > >          // NOTE: for things to generally work properly BigDecimal
> > should really be used as the java-type in the field type def
> > XML files
> > >          Object value = get(name);
> > >          if (value instanceof Double) {
> > > -            return new BigDecimal(((Double) value).doubleValue());
> > > +            return new BigDecimal(((Double) value).toString());
> > >          } else {
> > >              return (BigDecimal) get(name);
> > >          }
> > >
> >
> >
>

[David E Jones]

I think the point of the JavaDoc write up is that:

new BigDecimal(((Double) value).doubleValue()) != new BigDecimal(((Double) value).toString())

BUT

BigDecimal.valueOf(((Double) value).doubleValue()) == new BigDecimal(((Double) value).toString())

In other words, instead of using the BigDecimal double constructor, we should be using the BigDecimal.valueOf(double) method.

Whatever the case relying on this value without rounding can be dangerous... This may help a lot for most circumstances though, so I'll go ahead and throw it in (the valueOf variation to avoid the overhead of creating a String to throw away).

That is in SVN rev 576660.

-David


Jacopo Cappellato wrote:

> What do you think about the attached patch?
> I really think it should be committed to the trunk, especially after
> reading the following notes:
>
> http://java.sun.com/j2se/1.5.0/docs/api/java/math/BigDecimal.html#BigDecimal(double)
>
>
> However, I'd like to get feedback from you before committing it since it
> is a low level change.
>
> Jacopo
>
> PS: thanks to Martin Anderson from the patch and research around it.
>