On Wed, Jul 28, 2010 at 7:07 AM, Robbie Gemmell
<[email protected]> wrote:
> 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));
>

Agreed !

> 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?)

Yes when I looked at this last night I had the same question too.
I think this code could probably be moved to the abstract message delegate.

> 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. Ill raise a
> JIRA for that.

Cool.

> Robbie
>
> On 28 July 2010 03:17,  <[email protected]> wrote:
>> Author: rajith
>> Date: Wed Jul 28 02:17:29 2010
>> New Revision: 979933
>>
>> URL: http://svn.apache.org/viewvc?rev=979933&view=rev
>> Log:
>> QPID-2766
>> Instead of doing Double.valueOf(Float.valueOf(value)) , I changed it to use 
>> Double.valueOf(value).
>>
>> Modified:
>>    
>> qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/message/AMQMessageDelegate_0_10.java
>>
>> Modified: 
>> qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/message/AMQMessageDelegate_0_10.java
>> URL: 
>> http://svn.apache.org/viewvc/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/message/AMQMessageDelegate_0_10.java?rev=979933&r1=979932&r2=979933&view=diff
>> ==============================================================================
>> --- 
>> qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/message/AMQMessageDelegate_0_10.java
>>  (original)
>> +++ 
>> qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/message/AMQMessageDelegate_0_10.java
>>  Wed Jul 28 02:17:29 2010
>> @@ -599,7 +599,7 @@ public class AMQMessageDelegate_0_10 ext
>>         {
>>             try
>>             {
>> -                return Double.valueOf(getFloatProperty(propertyName));
>> +                return Double.valueOf(getStringProperty(propertyName));
>>             }
>>             catch(MessageFormatException e)
>>             {
>>
>>
>>
>> ---------------------------------------------------------------------
>> Apache Qpid - AMQP Messaging Implementation
>> Project:      http://qpid.apache.org
>> Use/Interact: mailto:[email protected]
>>
>>
>
> ---------------------------------------------------------------------
> Apache Qpid - AMQP Messaging Implementation
> Project:      http://qpid.apache.org
> Use/Interact: mailto:[email protected]
>
>



-- 
Regards,

Rajith Attapattu
Red Hat
http://rajith.2rlabs.com/

---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:[email protected]

Reply via email to