Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.0.9-core, 1.2.9-core
    • Component/s: None
    • Labels:
      None

      Description

      Due to a potential bug in BigDecimal there is a bug, when you use BigDecimal
      with a NumberConverter.

      Like
      <tr:inputText value="#

      {bean.number}

      " ...>
      <tr:convertNumber />
      </tr:inputText>

      For instance, when the entered value is "333.111" the actual stored value is 333.1109999999999899955582804977893829345703125

      There is a mathematic explanation for that in here:
      http://en.wikipedia.org/wiki/Floating_point#Accuracy_problems

        Issue Links

          Activity

          Hide
          Matthias Weßendorf added a comment -

          Here are two BigDecimals:
          -new java.math.BigDecimal(new Double(333.111).toString())
          -new java.math.BigDecimal(new Double(333.111).doubleValue())

          the last is "wrong" in the sense of why the bug was filed...

          The Trinidad Converter could do something like this, at the end of its
          getAsObject() method:

          ...
          ValueExpression ve = component.getValueExpression("value");

          if(ve != null &&
          BigDecimal.class.isAssignableFrom(ve.getType(context.getELContext())))

          { return (new BigDecimal(num.toString())); }

          ...

          However, this is (very) expensive, to execute on every time the getAsObject()
          from the NumberConverter is called. Since it always goes to the last guy, in the expression.

          Show
          Matthias Weßendorf added a comment - Here are two BigDecimals: -new java.math.BigDecimal(new Double(333.111).toString()) -new java.math.BigDecimal(new Double(333.111).doubleValue()) the last is "wrong" in the sense of why the bug was filed... The Trinidad Converter could do something like this, at the end of its getAsObject() method: ... ValueExpression ve = component.getValueExpression("value"); if(ve != null && BigDecimal.class.isAssignableFrom(ve.getType(context.getELContext()))) { return (new BigDecimal(num.toString())); } ... However, this is (very) expensive, to execute on every time the getAsObject() from the NumberConverter is called. Since it always goes to the last guy, in the expression.
          Hide
          Matthias Weßendorf added a comment -

          Got feedback from Blake Sullivan:

          I believe that the source of the problem is that we are always converting to
          Double, which can lose information. The safe thing to do is to use
          BigDecimal as our intermediate type, which is lossless.

          Show
          Matthias Weßendorf added a comment - Got feedback from Blake Sullivan: I believe that the source of the problem is that we are always converting to Double, which can lose information. The safe thing to do is to use BigDecimal as our intermediate type, which is lossless.
          Hide
          Matthias Weßendorf added a comment -

          I now noticed that there is a setParseBigDecimal( bool ) on DecimalFormat (which is used).
          Setting this to true is the fix. It may be a little bit expensive, but at the end, we are safe with this fix

          Show
          Matthias Weßendorf added a comment - I now noticed that there is a setParseBigDecimal( bool ) on DecimalFormat (which is used). Setting this to true is the fix. It may be a little bit expensive, but at the end, we are safe with this fix
          Hide
          Ed Burns added a comment -

          Hello Matthias,

          I made the change you suggested to my NumberConverter, but I cannot reproduce the error you're describing.

          Here is my testcase:

          Converter converter = application.createConverter("javax.faces.Number");

          String stringToConvert = "333.111";
          Object obj = converter.getAsObject(getFacesContext(), text,
          stringToConvert);
          assertTrue(obj instanceof java.lang.Number);
          String str = converter.getAsString(getFacesContext(), text, obj);
          assertTrue(str.equals(stringToConvert));

          The obj I get back is now a BigDecimal, after making your change, however, its value is still 333.111, not 333.1109999999999899955582804977893829345703125 .

          How can I reproduce this?

          Show
          Ed Burns added a comment - Hello Matthias, I made the change you suggested to my NumberConverter, but I cannot reproduce the error you're describing. Here is my testcase: Converter converter = application.createConverter("javax.faces.Number"); String stringToConvert = "333.111"; Object obj = converter.getAsObject(getFacesContext(), text, stringToConvert); assertTrue(obj instanceof java.lang.Number); String str = converter.getAsString(getFacesContext(), text, obj); assertTrue(str.equals(stringToConvert)); The obj I get back is now a BigDecimal, after making your change, however, its value is still 333.111, not 333.1109999999999899955582804977893829345703125 . How can I reproduce this?
          Hide
          Matthias Weßendorf added a comment -

          Hi Ed,

          the testcase doesn't reproduce it. Here is why:

          Converter converter = application.createConverter("javax.faces.Number");
          ...
          converter.getAsObject(getFacesContext(), text,
          stringToConvert);
          ...

          BEFORE your change, it would return a Double, with the Value 333.111, which is correct.
          Now, it returns a BigDecimal, which is more correct, but still the value is 333.111 (as wanted)

          But... when the bean value is actually BigDecimal, it (the Double object) would be converted into a BigDecmial,
          by EL engine with something like:
          new BigDecimal(doubleObjectReturnedFromTheConverter.doubleValue());

          this... would cause the problem. Therefore I provided these two simple tests in my first comment:
          -new java.math.BigDecimal(new Double(333.111).toString())
          -new java.math.BigDecimal(new Double(333.111).doubleValue())

          the last is "wrong" in the sense of why the bug was filed... (means a value like 333.110984654754...)

          So you can reproduce it an application such as:
          <tr:inputText value="#

          {bean.number}

          " ...>
          <tr:convertNumber />
          </tr:inputText>

          Where "number" property is actually BigDecimal and... the converter actually returns a double (which WAS the case).
          Is this more clear ?

          Show
          Matthias Weßendorf added a comment - Hi Ed, the testcase doesn't reproduce it. Here is why: Converter converter = application.createConverter("javax.faces.Number"); ... converter.getAsObject(getFacesContext(), text, stringToConvert); ... BEFORE your change, it would return a Double, with the Value 333.111, which is correct. Now, it returns a BigDecimal, which is more correct, but still the value is 333.111 (as wanted) But... when the bean value is actually BigDecimal, it (the Double object) would be converted into a BigDecmial, by EL engine with something like: new BigDecimal(doubleObjectReturnedFromTheConverter.doubleValue()); this... would cause the problem. Therefore I provided these two simple tests in my first comment: -new java.math.BigDecimal(new Double(333.111).toString()) -new java.math.BigDecimal(new Double(333.111).doubleValue()) the last is "wrong" in the sense of why the bug was filed... (means a value like 333.110984654754...) So you can reproduce it an application such as: <tr:inputText value="# {bean.number} " ...> <tr:convertNumber /> </tr:inputText> Where "number" property is actually BigDecimal and... the converter actually returns a double (which WAS the case). Is this more clear ?

            People

            • Assignee:
              Matthias Weßendorf
              Reporter:
              Matthias Weßendorf
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development