Qpid
  1. Qpid
  2. QPID-2766

string to double conversion results in questionable precision

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 0.6
    • Fix Version/s: 0.7
    • Component/s: Java Client
    • Labels:
      None

      Description

      The following test:

      public void testString2Double_1() {
      try

      { Message message = senderSession.createMessage(); message.setStringProperty("pi", "3.14159"); assertEquals(3.14159, message.getDoubleProperty("pi"),0); }

      catch (JMSException e)

      { fail(e); }

      }

      Fails with:

      junit.framework.AssertionFailedError: expected:<3.14159> but
      was:<3.141590118408203>

      This appears to be because the client ends up doing the equivalent of
      Double.valueOf(Float.valueOf(value)) when doing string to double conversion
      rather than the more direct Double.valueOf(value).

        Activity

        Hide
        Rajith Attapattu added a comment -

        Fixed and reviewed.

        Show
        Rajith Attapattu added a comment - Fixed and reviewed.
        Hide
        Rajith Attapattu added a comment -

        Thanks for catching this, I have fixed it.

        Show
        Rajith Attapattu added a comment - Thanks for catching this, I have fixed it.
        Hide
        Robbie Gemmell added a comment -

        Whilst I agree with the reason for making it I dont think the change made is entirely correct as it opens us to violating the JMS spec for property type conversion, since it will now first convert all property types (except Double obviously) to a String and then on to a Double. Only Floats, Doubles, and Strings are allowed to be read back as a Double value, everything else should throw a MessageFormatException. I would suggest the change should be something more along the lines of:

        if(o instanceof Double)

        { return ((Double)o).doubleValue(); }

        + else if(o instanceof String)
        +

        { + return Double.valueOf((String) o); + }

        else
        {
        try
        {
        return Double.valueOf(getFloatProperty(propertyName));

        In looking at this I have also come to notice that the 0-10 message property handling (do we really need to have seperate handler code for 0-8/9 and 0-10 for all of this?) seems to improperly convert String properties into short, int, and long by delegating the conversion down to the getByteProperty() method which then restricts it to using Byte.valueof(), which is rarely going to work correctly. Ive raised that as QPID-2770.

        Show
        Robbie Gemmell added a comment - Whilst I agree with the reason for making it I dont think the change made is entirely correct as it opens us to violating the JMS spec for property type conversion, since it will now first convert all property types (except Double obviously) to a String and then on to a Double. Only Floats, Doubles, and Strings are allowed to be read back as a Double value, everything else should throw a MessageFormatException. I would suggest the change should be something more along the lines of: if(o instanceof Double) { return ((Double)o).doubleValue(); } + else if(o instanceof String) + { + return Double.valueOf((String) o); + } else { try { return Double.valueOf(getFloatProperty(propertyName)); In looking at this I have also come to notice that the 0-10 message property handling (do we really need to have seperate handler code for 0-8/9 and 0-10 for all of this?) seems to improperly convert String properties into short, int, and long by delegating the conversion down to the getByteProperty() method which then restricts it to using Byte.valueof(), which is rarely going to work correctly. Ive raised that as QPID-2770 .

          People

          • Assignee:
            Rajith Attapattu
            Reporter:
            Rajith Attapattu
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development